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

Add setuptools_scm build arguments to pyproject.toml #110

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dihm
Copy link
Contributor

@dihm dihm commented Feb 23, 2024

This provides further fixes to how we use setuptools_scm as noted in comments on #106.

This should fix the random build issues (of which I still cannot replicate the successful build from two weeks ago). Basic premise is that setuptools_scm will use whatever is in the pyproject.toml to determine the version scheme if it is getting version by inspecting files (ie not installing). It will use setup.py to override the pyproject.toml settings when installing, and it will use __version__.py to override everything when importing the package.

There are a few minor choices we could make here beyond what is currently in the PR:

  1. The os.getenv calls in setup.py and __version__.py are no longer required. I'm not sure they are even needed with this change. Should we just remove them?
  2. setuptools_scm is a sort of runtime dependency of all the packages right now. If you have a .git folder, it will fail if setuptools_scm isn't installed. I'm inclined to wrap that in a try/except so it just falls back to importlib_metadata instead and keep this off the official dependency list. That said, we could also just enforce it as a dependency for everyone, even it they are installing from pip.
  3. We could use this as an opportunity to port over setup.cfg to the pyproject.toml and only have one metadata file.
  4. It may not be a bad idea to drop node-and-date from the local scheme when installing, (particularly for editable mode). Then we wouldn't need the overrides in setup.py which I suspect might be fragile going forward. It also makes the package versions under conda list a bit less noisy (so only have v3.3.0.devN instead of v3.3.0.devN+SHA.date) and doesn't remove much information since the node/date info in an editable install is all but worthless anyway for how quickly it stales. Having that info at import time where it can update automatically is the only place I see it having real value.

These changes, assuming they are sufficient, will need to be ported to all the repos. So @philipstarkey, any thoughts before I get cracking on that?

@dihm dihm added the bug Something isn't working label Feb 23, 2024
@dihm dihm self-assigned this Feb 23, 2024
@dihm
Copy link
Contributor Author

dihm commented Mar 1, 2024

@philipstarkey I've gone ahead and made commits that implement all of the above suggestions. Happy to back out any that are beyond the pail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant