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

Improve referencing __version__ attribute. #913

Open
jenstroeger opened this issue Nov 17, 2023 · 7 comments
Open

Improve referencing __version__ attribute. #913

jenstroeger opened this issue Nov 17, 2023 · 7 comments
Labels

Comments

@jenstroeger
Copy link
Contributor

Description

The other day I came across the package version declaration in SQLAlchemy’s setup.cfg here:

version = attr: sqlalchemy.__version__

The handling of special directive attr: is documented here.

I thought this quite useful to ensure that a version is declared in one place only; currently cz relies on version_files to point at attributes that need to be bumped.

This is probably a low-priority item that may or may not make sense. But I wanted to raise it for discussion anyway…

Possible Solution

No response

Additional context

No response

Additional context

No response

@woile
Copy link
Member

woile commented Nov 18, 2023

What about custom version providers?
I think you could have one using __version__ as the source of the version

@jenstroeger
Copy link
Contributor Author

Hmm. That’s a nice idea but I’d have to wire my own blob of code into into pyproject.toml and ship another module, right? Unless we allow folks to stash Python into a TOML file (but that’s getting ugly for little benefit…) 🤔

@woile
Copy link
Member

woile commented Nov 23, 2023

You'd have to publish a package with the custom provider in python, no code goes in pyproject.toml.

Then you'd install:

pip install commitizen cz-attr-version

And then you'd do in commitizen toml:

provider = "attr_version" 

@woile
Copy link
Member

woile commented Nov 23, 2023

Now that I think about it... maybe we could even make the existing provider pep621 read this, right?
pep621 is reading from the version field, that means it should be able to interpret accepted values, either a version or an attr 🤔

On the other hand, the recommendation seems to point to dropping the usage of any version information at module leven in favor of just using importlib.metadata.version()

see pep-396-rejection

Thoughts @Lee-W @noirbizarre ?

@jenstroeger
Copy link
Contributor Author

Now that I think about it... maybe we could even make the existing provider pep621 read this, right?

I do like that.

On the other hand, the recommendation seems to point to dropping the usage of any version information at module leven in favor of just using importlib.metadata.version() see pep-396-rejection

That PEP is from 2011, so I’m tempted to not give it too much weight. Might be worth bringing up for discussion at the forum?

@Lee-W Lee-W added the type: feature A new enhacement proposal label Dec 3, 2023
@Lee-W
Copy link
Member

Lee-W commented Dec 3, 2023

Now that I think about it... maybe we could even make the existing provider pep621 read this, right?

like this one as well

@noirbizarre
Copy link
Member

noirbizarre commented Dec 4, 2023

This is a complex topic I know quite well (sorry for the long post, but on this topic, this is required).

You need to include in the reasoning that while PEP621 is about exposing static metadata that can be read at packaging time (which is perfect for commitizen as it only imply reading/writing a toml file), both importlib.metadata.version() and <pkg>.__version__ are runtime ways of accessing the version, and I think this is a bit out of scope for commitizen.

Runtime access to both of those is not yet standardized, PEP621 is not about this and I don't think commitizen has any business to to on this. Those are the responsibility:

  • of the PEP517 build backend of your package manager to properly set the version as specified in Core Metadata at packing time (this is what you get using importlib.metadata)
  • of the developper to properly expose the version in <pkg>.__version__ (given this one is not specified at all and is more a convention)

Note that PEP517 build backend is what makes your project buildable with build and pip-installable from scm even if built with pdm, poetry or any other tool. It also allows proper packaging by other tools in the case a wheel does not exists (because it was never published or because the wheel is platform specific, and you use one for which no wheel has been published)

However, both importlib.metadata.version() and <root_module>.__version__ SHOULD reflect PEP621 versioning. Given the lack of PEP on the topic, there are many ways of handling this.

Here's my recipe to have commitizen properly works with PEP621 metadata and having both runtime access properly work (and my PR on version providers was meant to allow that):

  • In my package I actually use pdm as package manager with the dynamic = ["version"] as PEP621 dynamic versioning and pdm scm provider (which exist for setuptools, poetry... as they are all forks or copy-paste from the setuptools-scm plugin) which leads to:
    • having a generated version file containing either the scm tag, either a version derived from the last tag and last commit hash. This file should be added to .gitignore.
    • having the same version being injected in Core Metadata at build/packaging time
  • I use commitizen SCM version provider (and not the PEP621) to read the version from the scm too but don't try to write it anywhere as the produced tag will be enough
  • I have a snippet in my <pkg>.__init__.py which is trying to fetch the version from importlib.metadata and then fallback on reading the generated version file and then an hardcoded static version (0.0.0-dev to be the lowest possible version and never break any version sorting algorithm) to expose it on __version__.

This makes pyproject.toml looks like this:

[project]  # PEP621 standard project metadata
...
dynamic = ["version"]  # standard way of delegating dynamic fields resolution to the PEP517 backend

[build-system]  # PEP517 build backend
requires = ["pdm-backend"]
build-backend = "pdm.backend"

[tool.pdm.version] # Specific to pdm build backend
source = "scm"
write_to = "<pkg>/VERSION"

[tool.commitizen]
version_provider = "scm"

and the <pkg>/__init__.py looks like this:

from __future__ import annotations

from importlib.metadata import PackageNotFoundError, version
from pathlib import Path

def read_version() -> str:
    try:
        return str(version("<pkg-name>"))  # This is the PEP621 `project.name` not the root package
    except (PackageNotFoundError, ImportError):
        version_file = Path(__file__).parent / "VERSION"
        return version_file.read_text() if version_file.is_file() else "0.0.0.dev"

__version__ = read_version()  # should be failsafe

Why not just read the version file ?
Because SCM tags don't support PEP440 nor semver build/local identifiers (the +something part which can be set at package time on some package manager), the version file generated by the scm plugin use the build identifier to add a commitish+hash for non tagged version and the real build identifier is set in Core Metadata so importlib.metadata gives the version that was actually packaged, included all dynamic parts especially those that could be set in CI, while the version file give the version generated from the scm.

But this is only one way of doing it properly, my way according to my constraints (one being having runtime traceability of deployed assets versions, event if they are not tagged, which is often the case when you have some CI and you only bump versions you consider stable after trial). Note that this recipe also works with editable installation of your project.

This is very related to how you expect to use those. For many people (especially those coming from the JS world where this is the package.json "normal way"), only tagged versions are relevant and so PEP621 version provider is enough.

TLDR: the PEP621 provider should not read those packaging or runtime only information (meaning they exist at a time commitizen can't read) however documenting how it's possible to properly read this metadata at runtime and supports all those conventions in a standard Python package could be a great addition.

Note: a static tool trying to read the __version__ field should be considered as broken by design, as it is an unspecified runtime-only facility which might rely on a lot of dynamic behavior and will most of the time break on static access. However, setuptools actually loads the module to read the attribute, which works but leads to other chicken and eggs problems (example: this means your __init__.py should not have external imports as it is read before dependency resolution, at metadata extraction time, preventing you to use any root module facade pattern if you have external dependencies, so it might be better to expose version in a dedicated nested module like <pkg>._version and just import it in __init__.py but says to setuptools to read it from the nested module attribute <pkg>._version.__version__ to avoid triggering uninstalled imports).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants