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.

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

Miscellaneous packaging improvements #703

Merged
merged 19 commits into from
Apr 17, 2023
Merged

Miscellaneous packaging improvements #703

merged 19 commits into from
Apr 17, 2023

Conversation

khaeru
Copy link
Member

@khaeru khaeru commented Apr 13, 2023

  • Use pyproject.toml for packaging, instead of setup.{cfg,py}.
  • Replace usage of deprecated pkg_resources with importlib.metadata/importlib_metadata (Python 3.7 only).
  • Various housekeeping and clean-up of the CI workflows.
  • Use pytest-xdist to parallelize tests (n=2) on GHA runners; this shortens run time of individual jobs by up to 5 minutes.
  • Cherry-pick two commits (by @glatterf42 originally at Test & improve perfomance in .tools.add_year #494) adjusting for pandas 2.0.0.

How to review

Read the diff and note that the CI checks all pass.

PR checklist

  • Merge Miscellaneous updates for 2023-W15 ixmp#476, required to address an issue with the codecov dependency.
  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation. N/A
  • Update release notes.

@khaeru khaeru self-assigned this Apr 13, 2023
@khaeru khaeru force-pushed the enh/2023-W15 branch 3 times, most recently from ff40a95 to e62fe9d Compare April 14, 2023 08:22
@codecov
Copy link

codecov bot commented Apr 14, 2023

Codecov Report

Merging #703 (d2c461b) into main (03a0eb5) will decrease coverage by 0.5%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main    #703     +/-   ##
=======================================
- Coverage   94.9%   94.4%   -0.5%     
=======================================
  Files         42      43      +1     
  Lines       3366    3432     +66     
=======================================
+ Hits        3195    3241     +46     
- Misses       171     191     +20     
Impacted Files Coverage Δ
message_ix/__init__.py 100.0% <100.0%> (ø)
message_ix/core.py 95.1% <100.0%> (ø)
message_ix/testing/__init__.py 99.6% <100.0%> (ø)
message_ix/tests/test_macro.py 99.4% <100.0%> (ø)
message_ix/tests/test_tutorials.py 100.0% <100.0%> (ø)
message_ix/tools/add_year/__init__.py 66.2% <100.0%> (ø)

... and 79 files with indirect coverage changes

@khaeru khaeru force-pushed the enh/2023-W15 branch 2 times, most recently from 1480f23 to 72259cb Compare April 14, 2023 11:07
@khaeru khaeru requested a review from glatterf42 April 14, 2023 14:55
@khaeru
Copy link
Member Author

khaeru commented Apr 17, 2023

@glatterf42 #683 led to conflicts in test_tutorials.py; please wait for me to rebase & deconflict before reviewing.

@glatterf42
Copy link
Member

Sorry, I didn't realise it would. I'll review once you are done.

khaeru and others added 18 commits April 17, 2023 10:15
An updated pip is guaranteed by actions/setup-python.
…instead of the 3rd-party styfle/cancel-workflow-action.
This simplifies the workflow file, but has the same effect.
A subdirectory is no longer needed, since ixmp is not checked out.
…replacing deprecated pkg_resources.
- Remove max-complexity from "lint" CI workflow.
- Reduce max-complexity from 38 → 14; mark current exceptions FIXME.
- Sort [tool.*] sections.
- Exclude doc/* from mypy type checking.
- Reduce mypy overrides.
- Remove workaround for iiasa/ixmp#449.
- Remove outdated e-mail address; add maintainers.
Abandon pandas.append in favor of pandas.concat

Exclude .vscode/ from git tracking

Adapt sphinx status_iterator to DeprecationWarning
- Streamline definition of tutorial test cases.
- Use upper case for globals.
- Give --dist=loadgroup in "pytest" CI workflow.
Copy link
Member

@glatterf42 glatterf42 left a comment

Choose a reason for hiding this comment

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

Looks good to me, only one question and a few very minor remarks from my side.

.github/workflows/pytest.yaml Show resolved Hide resolved
message_ix/tests/test_tutorials.py Show resolved Hide resolved
message_ix/tests/test_tutorials.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Co-authored-by: Fridolin Glatter <[email protected]>
@khaeru
Copy link
Member Author

khaeru commented Apr 17, 2023

NB the codecov/patch check failure occurs because a few lines are changed in .tools.add_year, but that code remains untested. The tests will be added by #494, from which those changes were cherry-picked.

@glatterf42
Copy link
Member

Do we now have to remove those commits from #494 or does git recognize that we included them here already once we merge both?

@khaeru
Copy link
Member Author

khaeru commented Apr 17, 2023

Do we now have to remove those commits from #494 or does git recognize that we included them here already once we merge both?

Yes, they can be removed in a few steps:

  1. Check out that branch.
  2. git rebase -i HEAD~10 —replacing "10" with some number that goes back far enough to include those commits.
  3. Edit the TODO list in the editor that is opened. Put "d" or "drop" on the start of the line for those commits that have been cherry-picked; leave the rest the same.
  4. git rebase main; this will move the branch to the head of main, which already contains the cherry-picked commits.

Another way is to just do (1) and (4). During the rebase, git will complain, twice, that the cherry-picked commit "is (now) empty", because all changes they contained are already on main. It will prompt you to do git rebase --skip each time.

@khaeru khaeru merged commit a824be9 into main Apr 17, 2023
@glatterf42
Copy link
Member

This is mostly just FYI: I've applied your suggestion to #494 and ran (1) and (4) after updating my local main. The process was slightly different for me, then: git immediately told me that it skipped the two previously applied commits. It still found two merge conflicts, however, which I had to resolve manually (both about the beginning of the definition of the interpolate_1d function). After that, I had to force push the branch because my initial updates to main were not yet included in the upstream branch.

@khaeru
Copy link
Member Author

khaeru commented Apr 18, 2023

In this PR I forgot to adjust this line in the "nightly" CI workflow:

cache-dependency-path: "**/setup.cfg"

…to refer to pyproject.toml, mirroring the change made to the "pytest" workflow. As a result the workflow fails, e.g. https://github.com/iiasa/message_ix/actions/runs/4728750039

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants