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

docs: Set "version" in Sphinx configuration #837

Merged
merged 1 commit into from
Feb 3, 2022
Merged

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Jan 29, 2022

This will allow us to display the specific version for "stable" on Read
the Docs (once nextstrain/sphinx-theme#16 is
merged).

Also sets "release" as the Sphinx documentation says¹:

If you don’t need the separation provided between version and
release, just set them both to the same value.

¹ https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-release

This will allow us to display the specific version for "stable" on Read
the Docs (once <nextstrain/sphinx-theme#16> is
merged).

Also sets "release" as the Sphinx documentation says¹:

    If you don’t need the separation provided between version and
    release, just set them both to the same value.

¹ https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-release
@@ -17,6 +17,7 @@

# -- Project information -----------------------------------------------------

from augur.__version__ import __version__ as augur_version
Copy link
Member

Choose a reason for hiding this comment

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

Probably not the right place but seeing this reminded me:

The __version__ PEP has been rejected a few months ago: https://www.python.org/dev/peps/pep-0396/#id14

Will we make the switch at some point to importlib.metadata.version(augur) as suggested: https://docs.python.org/3/library/importlib.metadata.html#distribution-versions

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, but the rejection standardizing __version__ has little bearing on independently using a variable called __version__ internally in our own code?

Copy link
Contributor

Choose a reason for hiding this comment

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

@corneliusroemer Could you create an issue for your recommended change as an enhancement? We can keep the current convention internally, but it would be nice to support the standard version interface, too.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think there's some confusion. We do support the standard version interface already. importlib.metadata.version() reads from the installed package metadata. In our case, we declare that in setup.py and during build it ends up in the source or wheel distribution metadata (e.g. nextstrain-augur-X.Y.Z/PKG-INFO or nextstrain_augur-X.Y.Z.dist-info/METADATA), which then gets installed alongside the code into site-packages (as *.dist-info/METADATA).

Copy link
Member Author

Choose a reason for hiding this comment

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

By way of example:

$ nextstrain shell /var/empty
Entering the Nextstrain build environment

Mapped volumes:
  /nextstrain/build is from /var/empty

Run the command "exit" to leave the build environment.

nextstrain:/nextstrain/build $ python3
Python 3.7.12 (default, Jan 26 2022, 22:28:48) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from importlib_metadata import version
>>> version('nextstrain-augur')
'13.1.2'

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's some confusion.

Yeah, thank you for clearing this up. I expected the following to work:

>>> from importlib.metadata import version
>>> version('augur')

since one always refers to Python packages in code by the actual modules exposed by a distribution and not the PyPI package name. But, I see now that this module is designed to get the version of a distribution.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's a subtly. You can use packages_distributions() (and maybe other ways) to map packages (not modules) → distribution, e.g.

>>> from importlib_metadata import packages_distributions, version
>>> [version(dist) for dist in packages_distributions()['augur']]
['13.1.2']

Copy link
Member Author

Choose a reason for hiding this comment

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

(and if need to go module → distribution, you do so by way of module.__package__, e.g. augur.io.__package__ == 'augur').

@tsibley tsibley merged commit 0268940 into master Feb 3, 2022
@tsibley tsibley deleted the trs/docs/set-version branch February 3, 2022 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

4 participants