#432 Use importlib.metadata.version to get the version
#502
Conversation
for more information, see https://pre-commit.ci
|
Some tests in Alternatives are to try to use |
|
See also: #491 |
|
I'm not sure how I feel about this whole"install towncrier along with your package" -- it kinda seems like a misfeature / attractive nuisance for the reasons we've just seen. :-/ I would argue for deprecation and adding ways to extract the version from the outside (pyproject.toml / extract using regexp from a file / run a command [e.g. hatch]). |
|
It would be much easier to use a |
|
It’s not that simple: hatch supports dynamic versions using plugins like I for one would claim that the user can run Don’t y’all feel too that these features are children of a bygone Python packaging era when things were simpler and more homogenous? |
|
I think that At the same time, I think that we should make simple things simple. So if the version is already defined as a simple text value inside the .toml file, I think that it makes sense to accept a PR in which the version is automatically extracted. We can document that this only works for limited use cases. I prefer to have all the common commands stored in file. If towncrier or if someone already has the package installed, they can use something like These options will not work for any case... but I don't think that we should reject a PR adding support for a specific use case. I expect that the majority of project will try to keep things simple, and have the version as a string inside the Just my feedback |
|
Hmm either way I'm sorry, I think the approach in this PR is the more correct one even though I disagree with the feature as a whole. It will need something like this and a conditional dependency Please consider this un-bikeshedded otherwise. |
|
So for this PR to be merged it needs:
I think there are already some tests generating With the automated tests, there is one big problem, if you start a python interpreter/process and while the process is running you install packages... the metadata of the new pacage might not be available. For my code sometimes I am calling It works... if in the same process I call So just something to take into consideration when testing. |
|
I triggered the current test suite to see if there are any regression that might be introduced by this change. To me, this approach is harmeless ... if you don't have the package install it should work as before. Also, I don't know if we should worry about python 3.7 ... It will be EOL soon. |
|
How about we (read: you 😇) cut a release and drop 3.7 afterwards? (Pls someone merge my typo PR first tho 😅) |
|
Dropping Python 3.7 before this makes a lot of sense, though the backport of |
|
Well... I think that the overall feedback is that people are expecting from Using So the direction of this PR is good. But in order to merge this PR, we need all tests to be green. |
|
Oh yes, I just saw that. I meant that I could wait until the next release until 3.7 support is dropped as @hynek suggests, and then work on it |
src/towncrier/_project.py
Outdated
|
|
||
| version = getattr(module, "__version__", None) | ||
| try: | ||
| version = metadata_version(f"{module}") |
There was a problem hiding this comment.
The str of a module is not something you can pass to importlib.metadata.version because you get a repr-like string:
>>> module = importlib.import_module("towncrier")
>>> print(f"{module}")
<module 'towncrier' from '/Users/.../site-packages/towncrier/__init__.py'>which is probably just a small mistake here, but it reminds me that in fact version() does not accept an Import Package name at all, but rather a Distribution Package. They're commonly the same but not necessarily so, to use some popular examples:
| Import Package | Distribution Package |
|---|---|
| attr1 | attrs |
| yaml | PyYAML |
As an example, let's say I'm the PyYAML developer, and I'm using towncrier with a __version__ variable via package = "yaml". If I want to drop that variable and specify my version in pyproject.toml instead, then we can't pass yaml to importlib.metadata.version. We have to load a mapping from one to the other:
>>> import importlib.metadata
>>> importlib.metadata.packages_distributions()['yaml']
['PyYAML']
>>> importlib.metadata.version('PyYAML')
'6.0.1'which in the context of get_version would be something like:
dist_packages = distributions_packages()
try:
dist_package = dist_packages[package]
except KeyError:
# do fallback to package.__version__ here
else:
if len(dist_package) > 1:
raise Exception("I found more than one package")
version = metadata_version(dist_package[0])So that's how we'd make towncrier use importlib.metadata in a backwards-compatible way (user doesn't have to change their tool.towncrier config).
It's a little unfortunate, though, that I have to do this:
[project]
name = "PyYAML"
version = "1.2.3"
[tool.towncrier]
package = "yaml"The version associated with the distribution package is right there, but then I have to tell towncrier one of the import packages I have so that it can look up the name of my distribution package.
PEP 621 says that the project.name is required to be statically defined, which means towncrier could try to read project.name by default if package is not defined and --version is not passed.
Notably Poetry doesn't yet support PEP 621 but the package would still be there for Poetry users.
Footnotes
-
attrs also has its newer
attrsnamespace of course :-) ↩
|
I realise that this PR has been lost in the hallows of my GitHub account... and it has taken almost an year for me to get back to this, following the notification for the received review, thanks! 🙂 I will shall look to rebase this branch soon and finish this up, because it looks like other PRs are now depending on this (such as #491). |
|
So something like my last commit in this PR, @RazerM ? |
|
I think the message in the exception raised needs to be updated as well for the test to pass |
pyproject.toml
Outdated
| @@ -31,6 +31,7 @@ requires-python = ">=3.8" | |||
| dependencies = [ | |||
| "click", | |||
| "importlib-resources>=5; python_version<'3.10'", | |||
| "importlib-metadata==4.6.*; python_version<'3.10'", | |||
There was a problem hiding this comment.
I think that this needs a comment to explain why we are pinning on 4.6 version.
Why not use the latest version ?
There was a problem hiding this comment.
A better pin would be >=4.6 I guess (this is the version that had parity with Python 3.10).
Description
This PR updates the
get_version()method insrc/towncrier/_project.pyto useimportlib.metadata.version()[Fixes #432]Checklist
src/towncrier/newsfragments/. Describe yourchange and include important information. Your change will be included in the public release notes.
docs/tutorial.rstis still up-to-date.docs/cli.rstreflects those changes.docs/configuration.rstreflects those changes.