Navigation Menu

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

BLD: use setuptools_scm instead of versioneer #341

Merged
merged 15 commits into from May 27, 2021

Conversation

bocklund
Copy link
Collaborator

@bocklund bocklund commented May 26, 2021

setuptools_scm is the PyPA blessed single source versioning solution. We are moving away from versioneer so we don't have to vendor it ourselves and becaue it depends on distutils which is deprecated in Python 3.10.

  • Adds setuptools_scm as a runtime dependency so editable local installs can get the version dynamically
  • Remove versioneer vendored module and shipped files from MANIFEST.in. We can now properly use setuptools.build_meta as our build system instead of setuptools.build_meta:__legacy__.
  • Changes the version scheme to be strict PEP 440. For example, our current version as of a commit on this branch is 0.8.6.dev19+gf3765968. More version scheme details at https://github.com/pypa/setuptools_scm#default-versioning-scheme

Checklist

  • The documentation examples have been regenerated if the Jupyter notebooks in the examples/ have changed. To regenerate the documentation examples, run jupyter nbconvert --to rst --output-dir=docs/examples examples/*.ipynb from the top level directory)
  • If any dependencies have changed, the changes are reflected in the
    • setup.py
    • environment-dev.yml

I checked the generated sdist and verified that the version in PKG-INFO matches the the one seen by python setup.py --version. The wheels show the version correctly in the CI as well.

@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #341 (8a73bcb) into develop (9593ab5) will increase coverage by 5.44%.
The diff coverage is 81.81%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #341      +/-   ##
===========================================
+ Coverage    84.25%   89.70%   +5.44%     
===========================================
  Files           45       44       -1     
  Lines         4529     4255     -274     
===========================================
+ Hits          3816     3817       +1     
+ Misses         713      438     -275     
Impacted Files Coverage Δ
pycalphad/__init__.py 92.85% <81.81%> (-7.15%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9593ab5...8a73bcb. Read the comment docs.

@bocklund bocklund requested a review from richardotis May 26, 2021 15:08
@richardotis
Copy link
Collaborator

Thanks for getting this done. This is a good step forward for compatibility with the future of Python packaging. We eventually would like to eliminate the runtime dependency on setuptools_scm by writing a hardcoded version file which is distributed in the packaged version, but currently PEP 517 build isolation doesn't work with editable installs using pip install -e. . There are workarounds, but the real fix is PEP 660: https://www.python.org/dev/peps/pep-0660/
Support for PEP 660 in pip is in progress: pypa/pip#8212

So, for now, we can take the runtime import hit in packaged versions (for dev installs, it'll be the same) and eventually make the switch to hardcoded version files when PEP 660 is implemented.

@bocklund
Copy link
Collaborator Author

bocklund commented May 26, 2021

I think the __init__.py implementation needs a small change to give root and relative_to arguments so that the dynamic versioning works for editable installs if you are outside the package directory.

To reproduce the issue I'm talking about

  1. Install pycalphad as editable
  2. Check that the version makes sense python -c 'import pycalphad; print(pycalphad.__version__)'
  3. Change to the directory of another git repository that has some git tagged versions
  4. Verify the issue, that the version check prints relative to the CWD's git repo tags python -c 'import pycalphad; print(pycalphad.__version__)'

@bocklund
Copy link
Collaborator Author

The step didn't work in the tests. Not sure yet how to configure it to prefer the git lookup, but fall back on the version from the package metadata.

The bare get_version() seems to be able to look at the git version of the current directory or the package metadata. It seems like setting the root and relative_to allowed us to get the git version even from another directory with it's own git, but it is not able to find the package metadata.

Not sure what the correct API usage for get_version should be, and it's not well documented.

Worst case, we do a try-except?

@richardotis
Copy link
Collaborator

I think get_version() accepts keyword arguments corresponding to the entries in our pyproject.toml, and when we call it without those arguments, they are ignored.

@bocklund
Copy link
Collaborator Author

Switched to a slightly different try/except scheme suggested by @richardotis.

The idea is that we use this trick from StackOverflow to use a file that isn't packaged as a hack for detecting if a user has a local checkout from an SCM system. If this file is present, the user has a local checkout/editable install, then we can get the version from the SCM using setuptools_scm. If the file is not present (i.e. regular installs from an sdist or a wheel), then the version c

This adds importlib_metadata as an explicit dependency until we support Python 3.8 or later when importlib.metadata becomes part of the stdlib.

@richardotis
Copy link
Collaborator

References for astropy's approach: astropy/astropy#10774 astropy/astropy#10831
Explanation for why we use a directory, instead of a single file, to exclude in MANIFEST.in: astropy/astropy#10774 (comment) pypa/setuptools#511 (comment)

@richardotis
Copy link
Collaborator

Can the dependency for importlib_metadata be updated so it's only included for Python <3.8?

@bocklund
Copy link
Collaborator Author

Can the dependency for importlib_metadata be updated so it's only included for Python <3.8?

In the setup.py?

@richardotis
Copy link
Collaborator

I was thinking in the setup.py and in the environment definition for the dev environment.

@richardotis
Copy link
Collaborator

richardotis commented May 26, 2021

It's also okay if they're just marked with a comment so we don't forget in ~2 years when Py37 support ends.

@richardotis richardotis self-requested a review May 26, 2021 23:29
@richardotis
Copy link
Collaborator

Getting some py37 errors at build-time, but everything seems to still build and work:

Collecting numpy==1.14.5; python_version == "3.7" and platform_machine != "aarch64" and platform_system != "AIX" and platform_python_implementation != "PyPy"
  Using cached numpy-1.14.5-cp37-none-win_amd64.whl (13.4 MB)
ERROR: scipy 1.6.3 has requirement numpy<1.23.0,>=1.16.5, but you'll have numpy 1.14.5 which is incompatible.

@richardotis
Copy link
Collaborator

Changing oldest-supported-numpy to numpy<1.20 in the pyproject.toml build requirements seems to work locally for me on Windows py37.

NumPy<1.20 seems ABI backwards compatible back to most reasonable versions (incl. 1.13, our minimum) and is guaranteed forward compatible.
@bocklund
Copy link
Collaborator Author

Things seemed to pass in reasonable time with the new pin. Do you want to give one last pass, @richardotis?

@bocklund bocklund requested a review from richardotis May 27, 2021 01:50
@richardotis
Copy link
Collaborator

Editable mode seems to be working locally without errors, and it's still compatible with python setup.py develop so I don't have to pay the build isolation cost if I choose not to. Editable mode seems to work in my dirty Py37 working environment as well as a fresh Py38 environment.

@bocklund
Copy link
Collaborator Author

Not to suggest changing this yet again, but I think the editable workaround we are using would enable using the hard coded write_to mechanism instead of the import lib mechanism.

@richardotis
Copy link
Collaborator

Open to it, but let's make it a separate PR.

@bocklund
Copy link
Collaborator Author

I also tested a local development install manually as well as pip install git+https://github.com/pycalphad/pycalphad.git@8a73bcb04c2627fb5793c626183ba0b244aa7d86

@bocklund bocklund merged commit 0964620 into pycalphad:develop May 27, 2021
bocklund added a commit that referenced this pull request May 28, 2021
- Sphinx `conf.py`: Don't load the version from a hardcoded `_version.py` (changes from #341)
- CI: Build (but don't deploy) documentation on pull requests so we can catch regressions in the docs in the future
- CI: Use pip to install rather than conda in docs builds
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.

None yet

2 participants