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

Switching from setup.py to pyproject.toml #1409

Merged
merged 23 commits into from
Sep 26, 2023
Merged

Switching from setup.py to pyproject.toml #1409

merged 23 commits into from
Sep 26, 2023

Conversation

john-science
Copy link
Member

@john-science john-science commented Sep 13, 2023

What is the change?

Here we are switching from setup.py to pyproject.toml, but the new pyproject.toml file actually replaces:

  • MANIFEST.in
  • pytest.ini
  • requirements-docs.txt
  • requirements-testing.txt
  • requirements.txt
  • ruff.toml
  • setup.py

Please notice this means that the version of ARMI is now defined in pyproject.toml not armi/meta.py. So the code has to get the version information from importlib.

Why is the change being made?

The Python Org is forcing our hand on this. The official Python packaging docs now recommend switching to pyproject.toml. So this will close #1124.

However, the new solution is certainly more modern. I like that we are centralizing so many thing into one place.


Checklist

  • This PR has only one purpose or idea.
  • Tests have been added/updated to verify that the new/changed code works.
  • The release notes (location doc/release/0.X.rst) are up-to-date with any important changes.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in setup.py.

@john-science john-science added the architecture Issues related to big picture system architecture label Sep 13, 2023
@john-science
Copy link
Member Author

john-science commented Sep 13, 2023

NOTE: I ran the docs GitHub Action before this PR and they worked. Just FYI.

@john-science
Copy link
Member Author

@drewj-usnctech @sombrereau I don't expect this to affect you. But, just in case, I thought you'd want a head's up.

@drewj-usnctech
Copy link
Contributor

👍 😻

Copy link
Member

@albeanth albeanth left a comment

Choose a reason for hiding this comment

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

A few comments to consider. Though I'm well out of my wheelhouse with providing an actual review!

pyproject.toml Show resolved Hide resolved
requirements-testing.txt Show resolved Hide resolved
pytest.ini Show resolved Hide resolved
doc/user/user_install.rst Outdated Show resolved Hide resolved
Copy link
Member

@albeanth albeanth left a comment

Choose a reason for hiding this comment

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

I'll approve, but someone should provide a secondary review as I'm not particularly well versed in this section of ARMI....

Copy link
Member

@opotowsky opotowsky left a comment

Choose a reason for hiding this comment

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

I love this so much.

Tony ended up getting you to add the docs to the toml, which was a great addition. I really can't think of anything missing here. I eyeball checked that the dependencies and old MANIFEST.in files matched. All looks great to me!

@john-science john-science merged commit aeac447 into main Sep 26, 2023
21 checks passed
@john-science john-science deleted the pyproj branch September 26, 2023 16:19
albeanth pushed a commit to albeanth/armi that referenced this pull request Oct 11, 2023
Here we are switching from `setup.py` to `pyproject.toml`, but the new `pyproject.toml` file actually replaces:

* `MANIFEST.in`
* `pytest.ini`
* `requirements-docs.txt`
* `requirements-testing.txt`
* `requirements.txt`
* `ruff.toml`
* `setup.py`

Please notice this means that the version of ARMI is now defined in `pyproject.toml` not `armi/meta.py`. So the code has to get the version information from `importlib`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture Issues related to big picture system architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We should switch to a pyproject.toml
4 participants