Modernize build system using PEP621 semantics

zoj613 commented 1 year ago

This commit replaces setup.cfg+setup.py with pyproject.toml and setuptools. It also removes versioneer with setuptools-scm for automatic package versioning during development and release.

Thank you for opening a PR!

Here are a few important guidelines and requirements to check before your PR can be merged:

[x] There is an informative high-level description of the changes.
[ ] The description and/or commit message(s) references the relevant GitHub issue(s).
[x] pre-commit is installed and set up.
[x] The commit messages follow these guidelines.
[x] The commits correspond to relevant logical changes, and there are no commits that fix changes introduced by other commits in the same branch/BR. If your commit description starts with "Fix...", then you're probably making this mistake.
[ ] There are tests covering the changes introduced in the PR.

Don't worry, your PR doesn't need to be in perfect order to submit it. As development progresses and/or reviewers request changes, you can always rewrite the history of your feature/PR branches.

If your PR is an ongoing effort and you would like to involve us in the process, simply make it a draft PR.

zoj613 commented 1 year ago

This is the resultant source distribution tree:

aehmc-0.0.12.dev4+gd54e2d0.d20230505/
aehmc-0.0.12.dev4+gd54e2d0.d20230505/LICENSE
aehmc-0.0.12.dev4+gd54e2d0.d20230505/MANIFEST.in
aehmc-0.0.12.dev4+gd54e2d0.d20230505/Makefile
aehmc-0.0.12.dev4+gd54e2d0.d20230505/PKG-INFO
aehmc-0.0.12.dev4+gd54e2d0.d20230505/README.md
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/__init__.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/_version.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/algorithms.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/hmc.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/integrators.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/mass_matrix.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/metrics.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/nuts.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/proposals.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/step_size.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/termination.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/trajectory.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/utils.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc/window_adaptation.py
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc.egg-info/
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc.egg-info/PKG-INFO
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc.egg-info/SOURCES.txt
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc.egg-info/dependency_links.txt
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc.egg-info/requires.txt
aehmc-0.0.12.dev4+gd54e2d0.d20230505/aehmc.egg-info/top_level.txt
aehmc-0.0.12.dev4+gd54e2d0.d20230505/pyproject.toml
aehmc-0.0.12.dev4+gd54e2d0.d20230505/requirements.txt
aehmc-0.0.12.dev4+gd54e2d0.d20230505/setup.cfg

versus the tree of the latest release on Pypi:

aehmc-0.0.11/
aehmc-0.0.11/LICENSE
aehmc-0.0.11/MANIFEST.in
aehmc-0.0.11/PKG-INFO
aehmc-0.0.11/README.rst
aehmc-0.0.11/aehmc/
aehmc-0.0.11/aehmc/__init__.py
aehmc-0.0.11/aehmc/_version.py
aehmc-0.0.11/aehmc/algorithms.py
aehmc-0.0.11/aehmc/hmc.py
aehmc-0.0.11/aehmc/integrators.py
aehmc-0.0.11/aehmc/mass_matrix.py
aehmc-0.0.11/aehmc/metrics.py
aehmc-0.0.11/aehmc/nuts.py
aehmc-0.0.11/aehmc/proposals.py
aehmc-0.0.11/aehmc/step_size.py
aehmc-0.0.11/aehmc/termination.py
aehmc-0.0.11/aehmc/trajectory.py
aehmc-0.0.11/aehmc/utils.py
aehmc-0.0.11/aehmc/window_adaptation.py
aehmc-0.0.11/aehmc.egg-info/
aehmc-0.0.11/aehmc.egg-info/PKG-INFO
aehmc-0.0.11/aehmc.egg-info/SOURCES.txt
aehmc-0.0.11/aehmc.egg-info/dependency_links.txt
aehmc-0.0.11/aehmc.egg-info/not-zip-safe
aehmc-0.0.11/aehmc.egg-info/requires.txt
aehmc-0.0.11/aehmc.egg-info/top_level.txt
aehmc-0.0.11/setup.cfg
aehmc-0.0.11/setup.py
aehmc-0.0.11/versioneer.py

zoj613 commented 1 year ago

Test failures are caused by #96

maresb commented 1 year ago

Hey, this looks good at first glance. I'm headed off to bed. I can take a closer look tomorrow.

Be warned that there are some really weird things that can go wrong here. What I'd do to check thoroughly is to build both the sdist and the wheel from before and after. Then do a directory comparison (I use Meld), and check that the before and after versions are equivalent. It's a good amount of work, but it's actually necessary if you want to be thorough.

zoj613 commented 1 year ago

@brandonwillard black pre-commit hook is failing the tests due to lines being too long. What do we do about these? Increase the line limit or follow black's suggestions?

brandonwillard commented 1 year ago

@brandonwillard black pre-commit hook is failing the tests due to lines being too long. What do we do about these? Increase the line limit or follow black's suggestions?

It looks like some of the config options in the old setup.cfg aren't being picked up in the new setup. Regarding black, that could be due to a new version and/or the aforementioned.

zoj613 commented 1 year ago

@brandonwillard black pre-commit hook is failing the tests due to lines being too long. What do we do about these? Increase the line limit or follow black's suggestions?

It looks like some of the config options in the old setup.cfg aren't being picked up in the new setup. Regarding black, that could be due to a new version and/or the aforementioned.

I think this is due to the fact that there aren't any hook settings for black specified in the pre-commit config and there is no black tool section in the pyproject.toml specifying black specific settings to override black's default settings. So we probably need to decide on sensible settings to add in pyproject.toml

zoj613 commented 1 year ago

Thanks for the review @maresb , I've added the suggestions in ec01897 (#97), please do take another look when you can.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (d4d6198) 100.00% compared to head (afc25f7) 100.00%.

:exclamation: Current head afc25f7 differs from pull request most recent head 12312d7. Consider uploading reports for the commit 12312d7 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #97 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 13 13 Lines 544 544 Branches 31 31 ========================================= Hits 544 544 ``` | [Impacted Files](https://app.codecov.io/gh/aesara-devs/aehmc/pull/97?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs) | Coverage Δ | | |---|---|---| | [aehmc/window\_adaptation.py](https://app.codecov.io/gh/aesara-devs/aehmc/pull/97?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs#diff-YWVobWMvd2luZG93X2FkYXB0YXRpb24ucHk=) | `100.00% <ø> (ø)` | | | [aehmc/\_\_init\_\_.py](https://app.codecov.io/gh/aesara-devs/aehmc/pull/97?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs#diff-YWVobWMvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

zoj613 commented 1 year ago

@brandonwillard @rlouf any particular comments before merge? I'll squash the 7 commits into one If there aren't any.

brandonwillard commented 1 year ago

@brandonwillard @rlouf any particular comments before merge? I'll squash the 7 commits into one If there aren't any.

Yeah, squash and merge is fine.

zoj613 commented 1 year ago

@brandonwillard @rlouf any particular comments before merge? I'll squash the 7 commits into one If there aren't any.

Yeah, squash and merge is fine.

done! I don't have merge permissions for aehmc so please do the merge once the CI run is done.

brandonwillard commented 1 year ago

@brandonwillard @rlouf any particular comments before merge? I'll squash the 7 commits into one If there aren't any.

Yeah, squash and merge is fine.

done! I don't have merge permissions for aehmc so please do the merge once the CI run is done.

One second, I'll add you to the group.

brandonwillard commented 1 year ago

Done; try it now.

zoj613 commented 1 year ago

Done; try it now.

merging is still blocked. Not sure if another setting needs to be changed.

maresb commented 1 year ago

Looks like a second review is required?

Edit: Ah, nevermind, I now think it's just because it was because the review was requested.

brandonwillard commented 1 year ago

OK, now?

brandonwillard commented 1 year ago

Much appreciated, @zoj613!

aesara-devs / aehmc

Modernize build system using PEP621 semantics #97

Codecov Report