Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Package OSeMOSYS Step #81

Merged
merged 17 commits into from
Jul 10, 2024
Merged

Package OSeMOSYS Step #81

merged 17 commits into from
Jul 10, 2024

Conversation

willu47
Copy link

@willu47 willu47 commented Oct 19, 2023 •
edited
Loading

This pull request is a work in progress.

I have made a start of packaging the OSeMOSYS Step code so that it can be published on PyPI and installed using pip.

I decided to use hatchling as the tool to assist with packaging, as it provides a number of useful utilities, including development environment management. It's also very lightweight - only requiring the use of a pyproject.toml file, so does not add a lot of custom boilerplate code to the repository. I'm adding notes to the readme as I go so that the less intuitive parts of hatchling are recorded.

To do

  • Write more tests
  • Define command line entry point to the package
  • Full lint and stylistic checks - possibly use git hooks to enforce (e.g. pre-commit)
  • Set up continuous deployment to PyPI
  • Set up continuous integration testing

@willu47 willu47 added the enhancement New feature or request label Oct 19, 2023
@willu47 willu47 requested a review from HauHe October 19, 2023 11:12
@willu47 willu47 self-assigned this Oct 19, 2023
@willu47 willu47 changed the base branch from main to develop October 19, 2023 11:45
@willu47 willu47 linked an issue Oct 19, 2023 that may be closed by this pull request
Package OSeMOSYS_step #59
Open
@willu47 willu47 marked this pull request as ready for review March 25, 2024 13:50
HauHe
HauHe reviewed Apr 2, 2024
View reviewed changes
Copy link
Collaborator

@HauHe HauHe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been running Utopia with the latest version of OSeMOSYS_step. Things seem to work as they are supposed to. However, regarding the To do list from the beginning of the PR I'm not able to say if:

  • the tests in the package can be considered a continuous integration
  • the deployment to PyPI is continuous, I see there is a file called pyproject.toml is it related to that?

However, I see that:

  • there are more tests than before
  • a command line entry point has been defined
  • There seems to have been a stylistic check

Perhaps @trevorb1 you could have a look at the former two points?

@HauHe HauHe requested a review from trevorb1 April 2, 2024 15:06
@trevorb1
Copy link
Collaborator

trevorb1 commented Apr 2, 2024

@HauHe I believe I added the CI to do testing! A few notes:

  1. I have commented out the formatting/linting option from the testing. If you run formatting (hatch fmt or hatch fmt --linter if you just want linting), quite a lot is formatted, and quite a few warnings/errors are raised (see below). Unfortunately, I don't have time right now to address all of these. You can either leave as is without the linting, or address the errors and uncomment the linting action in the workflow.
  2. This does not do packaging of the project to PyPI.
  3. This only checks up to python version 3.11, as that is what has been specified in the project dependencies. If you want to update to python 3.12, you will need to update the pyproject.toml, I believe.

Here is the printout from hatch when formatting:

OSeMOSYS_step$ hatch fmt 
src/archive/main_ms.py:34:11: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/main_ms.py:36:22: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/main_ms.py:37:13: B007 Loop control variable `root` not used within loop body
src/archive/main_ms.py:37:19: B007 Loop control variable `dirs` not used within loop body
src/archive/main_ms.py:56:22: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/main_ms.py:63:13: T201 `print` found
src/archive/main_ms.py:74:16: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/main_ms.py:84:27: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/main_ms.py:87:27: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/main_ms.py:102:44: C408 Unnecessary `list` call (rewrite as a literal)
src/archive/main_ms.py:109:33: T201 `print` found
src/archive/main_ms.py:133:9: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
src/archive/main_ms.py:139:33: B007 Loop control variable `j` not used within loop body
src/archive/main_ms.py:147:13: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
src/archive/main_ms.py:169:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
src/archive/main_ms.py:181:13: T201 `print` found
src/archive/main_ms.py:216:121: E501 Line too long (536 > 120)
src/archive/main_ms.py:219:121: E501 Line too long (315 > 120)
src/archive/main_ms.py:220:121: E501 Line too long (171 > 120)
src/archive/main_ms.py:221:121: E501 Line too long (125 > 120)
src/archive/main_ms.py:223:121: E501 Line too long (198 > 120)
src/archive/main_ms.py:235:22: E711 Comparison to `None` should be `cond is None`
src/archive/main_ms.py:243:27: PLR2004 Magic value used in comparison, consider replacing 2 with a constant variable
src/archive/main_ms.py:261:28: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/main_ms.py:283:28: E711 Comparison to `None` should be `cond is not None`
src/archive/main_ms.py:294:44: S602 `subprocess` call with `shell=True` identified, security issue
src/archive/main_ms.py:303:121: E501 Line too long (130 > 120)
src/archive/main_ms.py:317:29: B007 Loop control variable `scn` not used within loop body
src/archive/main_ms.py:330:21: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
src/archive/main_ms.py:342:30: E711 Comparison to `None` should be `cond is not None`
src/archive/main_ms.py:353:44: S602 `subprocess` call with `shell=True` identified, security issue
src/archive/main_ms.py:366:121: E501 Line too long (121 > 120)
src/archive/main_ms.py:376:13: E741 Ambiguous variable name: `l`
src/archive/main_ms.py:386:28: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/main_ms.py:407:28: E711 Comparison to `None` should be `cond is not None`
src/archive/main_ms.py:418:44: S602 `subprocess` call with `shell=True` identified, security issue
src/archive/main_ms.py:427:121: E501 Line too long (130 > 120)
src/archive/main_ms.py:441:29: B007 Loop control variable `scn` not used within loop body
src/archive/main_ms.py:454:21: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
src/archive/main_ms.py:466:28: E711 Comparison to `None` should be `cond is not None`
src/archive/main_ms.py:477:44: S602 `subprocess` call with `shell=True` identified, security issue
src/archive/main_ms.py:490:121: E501 Line too long (121 > 120)
src/archive/main_step.py:25:9: T201 `print` found
src/archive/main_step.py:32:69: UP031 Use format specifiers instead of percent format
src/archive/main_step.py:32:121: E501 Line too long (141 > 120)
src/archive/main_step.py:33:21: S602 `subprocess` call with `shell=True` identified, security issue
src/archive/main_step.py:37:8: E721 Do not compare types, use `isinstance()`
src/archive/main_step.py:45:9: T201 `print` found
src/archive/main_step.py:60:13: T201 `print` found
src/archive/main_step.py:69:9: T201 `print` found
src/archive/main_step.py:84:13: T201 `print` found
src/archive/new_scen.py:3:8: F401 `os` imported but unused
src/archive/new_scen.py:16:121: E501 Line too long (232 > 120)
src/archive/new_scen.py:23:121: E501 Line too long (128 > 120)
src/archive/results_to_next_step.py:36:121: E501 Line too long (151 > 120)
src/archive/solv.py:1:121: E501 Line too long (129 > 120)
src/archive/solv.py:24:73: UP031 Use format specifiers instead of percent format
src/archive/solv.py:24:121: E501 Line too long (146 > 120)
src/archive/solv.py:25:20: S602 `subprocess` call with `shell=True` identified, security issue
src/archive/solv.py:31:34: ARG001 Unused function argument: `scen_info`
src/archive/solv.py:44:5: E722 Do not use bare `except`
src/archive/solv.py:58:13: ARG001 Unused function argument: `path_lp`
src/archive/solv.py:59:12: F821 Undefined name `path_sol`
src/archive/solv.py:62:15: ARG001 Unused function argument: `path_lp`
src/archive/solv.py:63:12: F821 Undefined name `path_sol`
src/archive/solv.py:87:28: E711 Comparison to `None` should be `cond is not None`
src/archive/step_to_final.py:14:20: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/step_to_final.py:34:21: C408 Unnecessary `dict` call (rewrite as a literal)
src/archive/step_to_final.py:51:9: T201 `print` found
src/archive/ts_gen.py:1:121: E501 Line too long (267 > 120)
src/archive/ts_gen.py:25:121: E501 Line too long (232 > 120)
src/archive/ts_gen.py:30:121: E501 Line too long (251 > 120)
src/archive/ts_gen.py:33:24: UP031 Use format specifiers instead of percent format
src/archive/ts_gen.py:33:121: E501 Line too long (129 > 120)
src/archive/ts_gen.py:36:24: UP031 Use format specifiers instead of percent format
src/archive/ts_gen.py:36:121: E501 Line too long (127 > 120)
src/archive/ts_gen.py:46:121: E501 Line too long (326 > 120)
src/archive/ts_gen.py:47:121: E501 Line too long (145 > 120)
src/archive/ts_gen.py:53:121: E501 Line too long (136 > 120)
src/archive/ts_gen.py:57:121: E501 Line too long (136 > 120)
src/osemosys_step/data_split.py:6:21: F401 `pathlib.Path` imported but unused
src/osemosys_step/data_split.py:7:20: F401 `typing.Any` imported but unused
src/osemosys_step/data_split.py:11:1: TID252 Relative imports are banned
src/osemosys_step/data_split.py:36:13: PLW2901 `for` loop variable `df` overwritten by assignment target
src/osemosys_step/data_split.py:66:27: E711 Comparison to `None` should be `cond is None`
src/osemosys_step/data_split.py:72:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
src/osemosys_step/data_split.py:81:121: E501 Line too long (144 > 120)
src/osemosys_step/data_split.py:110:25: PLR2004 Magic value used in comparison, consider replacing 2 with a constant variable
src/osemosys_step/data_split.py:122:25: PLR2004 Magic value used in comparison, consider replacing 2 with a constant variable
src/osemosys_step/data_split.py:132:33: E711 Comparison to `None` should be `cond is None`
src/osemosys_step/data_split.py:155:33: E711 Comparison to `None` should be `cond is None`
src/osemosys_step/data_split.py:163:33: E711 Comparison to `None` should be `cond is None`
src/osemosys_step/main.py:25:1: TID252 Relative imports are banned
src/osemosys_step/main.py:26:1: TID252 Relative imports are banned
src/osemosys_step/main.py:27:1: TID252 Relative imports are banned
src/osemosys_step/main.py:27:1: TID252 Relative imports are banned
src/osemosys_step/main.py:27:1: TID252 Relative imports are banned
src/osemosys_step/main.py:77:9: A001 Variable `dir` is shadowing a Python builtin
src/osemosys_step/main.py:82:9: A001 Variable `dir` is shadowing a Python builtin
src/osemosys_step/main.py:85:9: A001 Variable `dir` is shadowing a Python builtin
src/osemosys_step/main.py:88:9: A001 Variable `dir` is shadowing a Python builtin
src/osemosys_step/main.py:114:23: E711 Comparison to `None` should be `cond is None`
src/osemosys_step/main.py:115:121: E501 Line too long (128 > 120)
src/osemosys_step/main.py:184:121: E501 Line too long (144 > 120)
src/osemosys_step/main.py:194:121: E501 Line too long (152 > 120)
src/osemosys_step/main.py:343:17: T201 `print` found
src/osemosys_step/main.py:363:21: T201 `print` found
src/osemosys_step/main.py:440:121: E501 Line too long (123 > 120)
src/osemosys_step/main.py:479:121: E501 Line too long (122 > 120)
src/osemosys_step/main_utils.py:81:13: T201 `print` found
src/osemosys_step/main_utils.py:146:9: B007 Loop control variable `step` not used within loop body
src/osemosys_step/main_utils.py:245:86: FBT001 Boolean-typed positional argument in function definition
src/osemosys_step/main_utils.py:245:86: FBT002 Boolean default positional argument in function definition
src/osemosys_step/main_utils.py:245:121: E501 Line too long (124 > 120)
src/osemosys_step/main_utils.py:465:121: E501 Line too long (122 > 120)
src/osemosys_step/main_utils.py:606:26: EM101 Exception must not use a string literal, assign to variable first
src/osemosys_step/main_utils.py:611:121: E501 Line too long (142 > 120)
src/osemosys_step/preprocess_data.py:33:8: F401 `os` imported but unused
src/osemosys_step/preprocess_data.py:37:18: F401 `pandas` imported but unused
src/osemosys_step/preprocess_data.py:75:121: E501 Line too long (140 > 120)
src/osemosys_step/preprocess_data.py:97:121: E501 Line too long (126 > 120)
src/osemosys_step/preprocess_data.py:102:121: E501 Line too long (122 > 120)
src/osemosys_step/preprocess_data.py:155:28: F821 Undefined name `param_current`
src/osemosys_step/preprocess_data.py:156:45: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:158:53: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:160:28: F821 Undefined name `param_current`
src/osemosys_step/preprocess_data.py:161:45: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:163:41: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:165:28: F821 Undefined name `param_current`
src/osemosys_step/preprocess_data.py:171:59: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:173:28: F821 Undefined name `param_current`
src/osemosys_step/preprocess_data.py:179:61: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:181:28: F821 Undefined name `param_current`
src/osemosys_step/preprocess_data.py:182:51: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:184:121: E501 Line too long (185 > 120)
src/osemosys_step/preprocess_data.py:203:48: PLR2004 Magic value used in comparison, consider replacing 0.0 with a constant variable
src/osemosys_step/preprocess_data.py:204:49: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:205:53: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:206:49: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:213:48: PLR2004 Magic value used in comparison, consider replacing 0.0 with a constant variable
src/osemosys_step/preprocess_data.py:214:49: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:215:49: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:222:47: PLR2004 Magic value used in comparison, consider replacing 0.0 with a constant variable
src/osemosys_step/preprocess_data.py:223:51: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:224:49: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:231:47: PLR2004 Magic value used in comparison, consider replacing 0.0 with a constant variable
src/osemosys_step/preprocess_data.py:232:53: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:233:49: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:240:48: PLR2004 Magic value used in comparison, consider replacing 0.0 with a constant variable
src/osemosys_step/preprocess_data.py:241:55: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:242:49: C409 Unnecessary `list` literal passed to `tuple()` (rewrite as a `tuple` literal)
src/osemosys_step/preprocess_data.py:318:25: PLR2004 Magic value used in comparison, consider replacing 4 with a constant variable
src/osemosys_step/preprocess_data.py:320:9: T201 `print` found
src/osemosys_step/solve.py:6:8: F401 `sys` imported but unused
src/osemosys_step/solve.py:8:31: F401 `typing.Union` imported but unused
src/osemosys_step/solve.py:14:84: RUF013 PEP 484 prohibits implicit `Optional`
src/osemosys_step/solve.py:14:96: ARG001 Unused function argument: `csv_data`
src/osemosys_step/solve.py:14:106: RUF013 PEP 484 prohibits implicit `Optional`
src/osemosys_step/solve.py:14:121: E501 Line too long (125 > 120)
src/osemosys_step/solve.py:36:68: RUF013 PEP 484 prohibits implicit `Optional`
src/osemosys_step/solve.py:53:29: S602 `subprocess` call with `shell=True` identified, security issue
src/osemosys_step/solve.py:57:29: S602 `subprocess` call with `shell=True` identified, security issue
src/osemosys_step/utils.py:36:35: S506 Probable use of unsafe loader `UniqueKeyLoader` with `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`.
src/osemosys_step/utils.py:44:21: PLR2004 Magic value used in comparison, consider replacing 2 with a constant variable
src/osemosys_step/utils.py:85:68: RUF013 PEP 484 prohibits implicit `Optional`
src/osemosys_step/utils.py:102:9: T201 `print` found
src/osemosys_step/utils.py:114:54: RUF013 PEP 484 prohibits implicit `Optional`
src/osemosys_step/utils.py:132:14: A001 Variable `dir` is shadowing a Python builtin
tests/test_results_to_next_step.py:3:29: F401 `pytest.raises` imported but unused
Found 670 errors (501 fixed, 169 remaining).
No fixes available (62 hidden fixes can be enabled with the `--unsafe-fixes` option).

@HauHe
Copy link
Collaborator

HauHe commented Apr 3, 2024

thanks a lot for taking the time for this @trevorb1!
Ideally we should sort this out now, but right now I want to first get the runs for the paper done. So unless you have objections @willu47 , I would merge this PR and then we can finalise the packaging at a later point.

@willu47
Copy link
Author

willu47 commented May 21, 2024

Looks good to me @HauHe . Neither continuous integration or continuous deployment are necessary to merge this PR.

@willu47 willu47 changed the title Package OSeMOSYS Step [WIP] Package OSeMOSYS Step May 21, 2024
@HauHe HauHe merged commit 0f3a9bb into develop Jul 10, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HauHe HauHe HauHe left review comments

@trevorb1 trevorb1 Awaiting requested review from trevorb1

Assignees

@willu47 willu47

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Package OSeMOSYS_step
3 participants
@willu47 @trevorb1 @HauHe