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

WIP: "modernize" single-source versioning #1273

Closed

Conversation

ChrisBarker-NOAA
Copy link
Contributor

@ChrisBarker-NOAA ChrisBarker-NOAA commented Jul 10, 2023

The single-sourcing-package-version page is pretty out of date.

I've (started to) update it to reflect the current standard of practice:

I know this doc deliberately doesn't make a recommendation, but I think putting the "preferred" methods first, and noting the reasons you might not want to use the older methods will help newbies -- and I did put in a "no longer recommended" section -- up to the team to decide if we want to word it that strongly.

What I did:

  • Restructured it a bit with a few sections to group related methods

  • Put the declarative forms up front (setup.cfg and pyproject.toml)

  • Moved setuptools-vcs from the bottom.
    NOTE: I'm not sure how strongly we want to recommend that at this point, I've been trying it out, and it's not all that mature -- in particular, it only works well with "standard" setups -- which might be OK for anyone starting fresh, but it's a bit clunky for those of us with legacy code structures.

  • Put the methoids that use built-time and run-time version finding into "no longer recommended"

  • some copy editing

This reflects my personal opinion, so will need some editing / discussion, but the page really should be modernized!

@wimglenn
Copy link
Contributor

Related: #182

@flying-sheep
Copy link
Contributor

We could just remove that page.

__version__ is just a convention, importlib.metadata.version('distname') is short, standardized, and always works.

Maybe we should just recommend to not bother about __version__.

@abravalheri
Copy link
Contributor

abravalheri commented Jul 20, 2023

We could just remove that page.

__version__ is just a convention, importlib.metadata.version('distname') is short, standardized, and always works.

Maybe we should just recommend to not bother about __version__.

Hi @flying-sheep, @wimglenn, I agree with the idea that __version__ is just a convention and that at runtime if you want to know the version of something that was installed the best is to do importlib.metadata.version('distname').

But I don't think the value of this page comes from the fact it allows you to write __version__ in your Python file... For example, even if we simply rename __version__ in the examples to _my_UGLY_private_CONST_dont_USE_it_in_runtime, the documentation page still would hold have relevant information about the build process.

Some workflows imply in using a single source of truth for the version information. Maybe because it is creating a Python wrapper for an existing library written in a compiled language. Maybe because the devs want to use the VCS as single source of truth. Maybe there are other reasons (and here I am avoiding to judge if each one of these reasons is good or not)...

But my interpretation is that the value of this page comes from it giving the user options to access version information from different files during build time, not runtime. So I don't think removing it is the best option. Maybe the content needs adjustments to reflect that reality, but I see the value in the page existing somewhere.

We can argue that there is no canonical way of doing that across multiple backends and that therefore it should be moved away from the website into another one; I understand this point of view. But I don't think the best answer is "people should not be using __version__, so let's remove the page").

Comment on lines 15 to 42
One way of single sourcing is to specify the version somewhere in the source code, e.g. ``my_package/__init__.py``. Then it can be found at runtime with ``my_package.__version__``, and that same value can be used to set the version when building the package.

#. Declare to read the version from the source in ``setuptools.cfg`` or ``pyproject.toml``

With recent versions of setuptools (since 46.4.0), one can add a declaration
to the project's :file:`setup.cfg` file (replacing "package" with the import
name of the package):

.. code-block:: ini

[metadata]
version = attr: package.__version__

As of the release of setuptools 61.0.0, one can also specify the
version dynamically in the project's :file:`pyproject.toml` file.

.. code-block:: toml

[project]
name = "package"
dynamic = ["version"]

[tool.setuptools.dynamic]
version = {attr = "package.__version__"}

Please be aware that declarative config indicators, including the
``attr:`` directive, are not supported in parameters to
:file:`setup.py`.
Copy link
Contributor

@abravalheri abravalheri Jul 20, 2023

Choose a reason for hiding this comment

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

How about we rename __version__ to something less controversial. For example:

Suggested change
One way of single sourcing is to specify the version somewhere in the source code, e.g. ``my_package/__init__.py``. Then it can be found at runtime with ``my_package.__version__``, and that same value can be used to set the version when building the package.
#. Declare to read the version from the source in ``setuptools.cfg`` or ``pyproject.toml``
With recent versions of setuptools (since 46.4.0), one can add a declaration
to the project's :file:`setup.cfg` file (replacing "package" with the import
name of the package):
.. code-block:: ini
[metadata]
version = attr: package.__version__
As of the release of setuptools 61.0.0, one can also specify the
version dynamically in the project's :file:`pyproject.toml` file.
.. code-block:: toml
[project]
name = "package"
dynamic = ["version"]
[tool.setuptools.dynamic]
version = {attr = "package.__version__"}
Please be aware that declarative config indicators, including the
``attr:`` directive, are not supported in parameters to
:file:`setup.py`.
One way of single sourcing is to specify the version somewhere in the source code. Let's suppose you have a Python module (e.g. ``my_package/__init__.py``) that defines the project version as a constant (e.g. `VERSION = "0.42"`). Some build-backends allow you to use this value when building the package.
If you use `setuptools` as build-backend you can try one of the following:
#. Using the `version = attr:` in ``setup.cfg``:
With recent versions of setuptools (since 46.4.0), one can add a declaration
to the project's :file:`setup.cfg` file (replacing `my_package` with the import
name of the package and `VERSION` with the name of the relevant attribute):
.. code-block:: ini
[metadata]
version = attr: my_package.VERSION
#. Using a dynamic version in `pyproject.toml`:
As of the release of setuptools 61.0.0, one can also specify the
version dynamically in the project's :file:`pyproject.toml` file.
.. code-block:: toml
[project]
name = "package"
dynamic = ["version"]
[tool.setuptools.dynamic]
version = {attr = "package.VERSION"}
Please be aware that declarative config indicators, including the
``attr:`` directive, are not supported in parameters to
:file:`setup.py`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to have examples specific to a build back-end? Isn't there a way to stay agnostic? At the very least we could use tabs like in the packaging tutorial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be great -- I astarted with the previous document, and didn't add much, and I don't know any any other build back ends -- so I can't write them up, but it would be great to have it.

But this does point out what I thin is a gap in the spec -- wouldn't it be nice if there were a standard way to specify how to set yourversion other than hard coding it in a toml file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about we rename version to something less controversial.

__version__ may be controversial, but it's also a very widely used convension -- seems a bit silly to not use the most common convention as the example.

Copy link
Member

Choose a reason for hiding this comment

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

wouldn't it be nice if there were a standard way to specify how to set yourversion other than hard coding it in a toml file?

attr: is setuptools-specific, but dynamic = ["version"] allows all the PEP 517 build backends to generate the version during the build time. How this is done and how it's configured, it's always going to be specific to each build backend.
PEP 517 describes the communication interface between the parties but it doesn't mandate which backend-specific features should be implemented since it's out of the scope. What's the point of a standard that stays away from the implementation details if it'd demand each backend to implement the same mechanism? If that was the case, we wouldn't have the multitude of backends — just one.
Also, imagine a project being packaged is a wrapper around a Rust crate. And the version is in that crate. A packager uses a build backend specifically targeting Rust projects. If a backend is asked to implement loading the version from Python importables, it becomes impossible since that contradicts its design.

So the dynamic thing is really the extent of the standard that cares about packaging interoperability and doesn't have leaky abstractions.

Copy link
Member

Choose a reason for hiding this comment

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

How about we rename version to something less controversial.

__version__ may be controversial, but it's also a very widely used convension -- seems a bit silly to not use the most common convention as the example.

I agree — to me, anything else would be controversial. I'd keep it as it is. But we can add a .. tip:: admonition stating that "__version__ is widely used and is sometimes considered controversial".

Comment on lines 43 to 53


#. While the modern standard is to use declarative files (such as ``setup.cfg``),
with older versions of setuptools, you may need to add a version-reading
function to setup.py: Example adapted from (from `pip setup.py <https://github.com/pypa/pip/blob/main/setup.py#L11>`_)::

from pathlib import Path

def read(rel_path):
here = os.path.abspath(os.path.dirname(__file__))
with codecs.open(os.path.join(here, rel_path), 'r') as fp:
here = Path(__file__).parent.absolute()
with open(here / rel_path), 'r') as fp:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how much people need to rely on setuptools<46.4.0 nowadays. Maybe we can omit this section?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could replace this with the use case the dev needs to process a given file to extract the version string (e.g. the cases when you have a plain text file with several info or a file written in a different language?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I certainly think we could probably simply omit the old stuff -- though maybe, as it says in the text, it may still be found in older packages -- I know I still have some published that use the old techniques.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure how much people need to rely on setuptools<46.4.0 nowadays. Maybe we can omit this section?

Pip-users? Not many. This'd mostly hit downstream packagers that might be stuck with old pinned packages in their distros.

Copy link
Contributor

@abravalheri abravalheri left a comment

Choose a reason for hiding this comment

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

Hi @ChrisBarker-NOAA, I am not a maintainer in this repo, but thank you very much for spending time trying to improve the docs.

I added some comments/suggestions, that you might agree or disagree :P

@ChrisBarker-NOAA
Copy link
Contributor Author

ChrisBarker-NOAA commented Jul 22, 2023

we rename __version__ to something less controversial

Is __version__ controversial?

Putting an attribute into the module for the version number is somewhat controversial -- but I think there's' some consensus that if you do do that, it should be called __version__.

It is certainly the most common name used.

Copy link
Contributor

@sinoroc sinoroc left a comment

Choose a reason for hiding this comment

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

I refer to my comment. In short, I wonder if we wouldn't be better off just mentioning importlib.metadata.version(), how to use it, and saying that 3rd party tools (build back-end plugins) exist without naming them explicitly. I feel like such a document as it is now as well as this new modernized version belong outside of packaging.python.org.

Copy link
Contributor Author

@ChrisBarker-NOAA ChrisBarker-NOAA left a comment

Choose a reason for hiding this comment

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

I've addressed a number of the comments in an update to the PR.

source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
Comment on lines 15 to 42
One way of single sourcing is to specify the version somewhere in the source code, e.g. ``my_package/__init__.py``. Then it can be found at runtime with ``my_package.__version__``, and that same value can be used to set the version when building the package.

#. Declare to read the version from the source in ``setuptools.cfg`` or ``pyproject.toml``

With recent versions of setuptools (since 46.4.0), one can add a declaration
to the project's :file:`setup.cfg` file (replacing "package" with the import
name of the package):

.. code-block:: ini

[metadata]
version = attr: package.__version__

As of the release of setuptools 61.0.0, one can also specify the
version dynamically in the project's :file:`pyproject.toml` file.

.. code-block:: toml

[project]
name = "package"
dynamic = ["version"]

[tool.setuptools.dynamic]
version = {attr = "package.__version__"}

Please be aware that declarative config indicators, including the
``attr:`` directive, are not supported in parameters to
:file:`setup.py`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be great -- I astarted with the previous document, and didn't add much, and I don't know any any other build back ends -- so I can't write them up, but it would be great to have it.

But this does point out what I thin is a gap in the spec -- wouldn't it be nice if there were a standard way to specify how to set yourversion other than hard coding it in a toml file?

Comment on lines 43 to 53


#. While the modern standard is to use declarative files (such as ``setup.cfg``),
with older versions of setuptools, you may need to add a version-reading
function to setup.py: Example adapted from (from `pip setup.py <https://github.com/pypa/pip/blob/main/setup.py#L11>`_)::

from pathlib import Path

def read(rel_path):
here = os.path.abspath(os.path.dirname(__file__))
with codecs.open(os.path.join(here, rel_path), 'r') as fp:
here = Path(__file__).parent.absolute()
with open(here / rel_path), 'r') as fp:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I certainly think we could probably simply omit the old stuff -- though maybe, as it says in the text, it may still be found in older packages -- I know I still have some published that use the old techniques.

source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
Comment on lines 15 to 42
One way of single sourcing is to specify the version somewhere in the source code, e.g. ``my_package/__init__.py``. Then it can be found at runtime with ``my_package.__version__``, and that same value can be used to set the version when building the package.

#. Declare to read the version from the source in ``setuptools.cfg`` or ``pyproject.toml``

With recent versions of setuptools (since 46.4.0), one can add a declaration
to the project's :file:`setup.cfg` file (replacing "package" with the import
name of the package):

.. code-block:: ini

[metadata]
version = attr: package.__version__

As of the release of setuptools 61.0.0, one can also specify the
version dynamically in the project's :file:`pyproject.toml` file.

.. code-block:: toml

[project]
name = "package"
dynamic = ["version"]

[tool.setuptools.dynamic]
version = {attr = "package.__version__"}

Please be aware that declarative config indicators, including the
``attr:`` directive, are not supported in parameters to
:file:`setup.py`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about we rename version to something less controversial.

__version__ may be controversial, but it's also a very widely used convension -- seems a bit silly to not use the most common convention as the example.

@ChrisBarker-NOAA
Copy link
Contributor Author

I've addressed most of the comments. As I noted in #1276, I think it's important to update this page SOON -- even if it will change again in the not-too distant future. What's published now is really, really, out of date.


One way of single sourcing is to specify the version string somewhere in the source code, e.g. ``my_package/__init__.py``. Then it can be found at runtime with ``my_package.__version__``, and that same value can be used to set the version when building the package.

#. Declare to read the version string from the source in ``setuptools.cfg`` or ``pyproject.toml``
Copy link
Member

@webknjaz webknjaz Sep 7, 2023

Choose a reason for hiding this comment

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

Typo

Suggested change
#. Declare to read the version string from the source in ``setuptools.cfg`` or ``pyproject.toml``
#. Declare to read the version string from the source in :file:`setup.cfg` or :file:`pyproject.toml`.

Copy link
Contributor

Choose a reason for hiding this comment

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

The plan from setuptools side is to eventually deprecate setup.cfg, so it should not be mentioned. There's also spurious spacing here between "string from".

Copy link
Contributor

@sinoroc sinoroc Sep 8, 2023

Choose a reason for hiding this comment

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

The plan from setuptools side is to eventually deprecate setup.cfg,

Oh? Really? That's sad. I do not want to have a pyproject.toml file that is kilometers long. I like having one configuration file per tool. It is easier to find things. Maybe it would be good to have a convention like pyproject.d/setuptools.toml.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be good to have a convention like pyproject.d/setuptools.toml.

That sounds like a great standard to have!

However it'd require a huge push to create and support it by the various packaging tools. Could you make a proposal over at the peps repository?


#. While the modern standard is to use declarative files (such as ``setup.cfg``),
with older versions of setuptools, you may need to add a version-reading
function to setup.py: Example adapted from (from `pip setup.py <https://github.com/pypa/pip/blob/main/setup.py#L11>`_)::
Copy link
Member

Choose a reason for hiding this comment

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

Let's pin the URL since it may become 404 if pip ever removes this file from its main branch.

Suggested change
function to setup.py: Example adapted from (from `pip setup.py <https://github.com/pypa/pip/blob/main/setup.py#L11>`_)::
function to :file`setup.py`: Example adapted from (from `pip setup.py <https://github.com/pypa/pip/blob/6f3a7181b6bfa0e95d479b3269baa0e551ff2cb2/setup.py#L11>`_)::

here = os.path.abspath(os.path.dirname(__file__))
with codecs.open(os.path.join(here, rel_path), 'r') as fp:
here = Path(__file__).parent.absolute()
with open(here / rel_path), 'r') as fp:
Copy link
Member

Choose a reason for hiding this comment

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

Since this is already injecting pathlib, just use the high-level API instead:

Suggested change
with open(here / rel_path), 'r') as fp:
return (here / rel_path).read_text(encoding='utf-8')

----------------------------------------

Most projects use a Source Code Management System (SCM), such as git or mercurial to manage the code and also manage releases.
In order to keep the versioning of releases in sync with your package, the version string can be kept in the tags of a version control system, and automatically extracted with a tool such as
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In order to keep the versioning of releases in sync with your package, the version string can be kept in the tags of a version control system, and automatically extracted with a tool such as
In order to keep the versioning of releases in sync with your package, the version string can be kept in the tags of a version control system, and automatically extracted during the build, with a tool such as


.. code-block:: ini
.. NOTE2: Is setuptools_scm the only one now?
Copy link
Member

Choose a reason for hiding this comment

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

For setuptools, yes. Other backends have their own plugins. Like hatch-vcs (which actually wraps setuptools-scm). There's also dunamai but it's a generic project, that doesn't provide direct packaging integrations.


.. code-block:: toml
A few tools you could use, in no particular order, and not necessarily complete:
Copy link
Member

Choose a reason for hiding this comment

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

Twisted has their versioning own thing too. And there's also pbr that I don't regard well, in comes from the OpenStack ecosystem.


As of the release of setuptools 61.0.0, one can specify the
version dynamically in the project's :file:`pyproject.toml` file.
An external build tool can be used that manages the version string in both the SCM and source code, either directly or via an API:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
An external build tool can be used that manages the version string in both the SCM and source code, either directly or via an API:
An external build tool can be used that manages the version string in both the SCM and source code, either directly or via an API.

``attr:`` directive, are not supported in parameters to
:file:`setup.py`.
Place the value in a simple ``VERSION`` text file and have both
:file:`setup.py` and the project code read it.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if version = file: VERSION would work. Not sure if this helper works for the version attribute... @abravalheri do you know?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't work, you must specify in the dynamic section if you want to use that feature.

[tool.setuptools.dynamic]
version = {file = "VERSION"}

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't work, you must specify in the dynamic section if you want to use that feature.

Oh, I'm talking about setup.cfg, not pyproject.toml. It is processed differently.

Copy link
Contributor

Choose a reason for hiding this comment

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

It works in setup.cfg, but as previously commented the plan on setuptools side is deprecation of that file.

Comment on lines 102 to 103
with open(os.path.join(mypackage_root_dir, 'VERSION')) as version_file:
version = version_file.read().strip()
Copy link
Member

Choose a reason for hiding this comment

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

The above example already uses pathlib for consistency, this should too:

Suggested change
with open(os.path.join(mypackage_root_dir, 'VERSION')) as version_file:
version = version_file.read().strip()
version = (mypackage_root_dir / 'VERSION').read_text(encoding='utf-8').strip()

Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes your package is extracted to a filesystem. That's not a safe assumption.

Copy link
Member

Choose a reason for hiding this comment

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

Fair point. But then, open() still wouldn't work, right? This means the need for something like importlib.resources.files().

Though, I think that it's pretty much guaranteed to be on the file system at build time. The runtime, OTOH, might need the importlib helpers.

Copy link
Contributor

Choose a reason for hiding this comment

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

importlib.resources.files() is good, but then you need the importlib_resources port for supporting Python < 3.9 (conditional dependency), or a compatibility fallback to stdlib pkgutil. To do it properly/correctly with a static file in the package it gets complicated pretty fast.


def read(rel_path):
here = os.path.abspath(os.path.dirname(__file__))
with codecs.open(os.path.join(here, rel_path), 'r') as fp:
here = Path(__file__).parent.absolute()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why calling absolute method? __file__ is an absolute path already since Python 3.4.

Copy link
Member

Choose a reason for hiding this comment

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

Many confuse absolute() and resolve(): https://discuss.python.org/t/pathlib-absolute-vs-resolve/2573/2.
We should use the latter:

Suggested change
here = Path(__file__).parent.absolute()
here = Path(__file__).parent.resolve()

@wimglenn to answer your question, __file__ is not what's being unwound on this line — the parent directory is. Also, absolute and resolved are different concepts. Absolute just makes the path start with / so it's not relative. The resolution process follows symlinks, revealing actual location in the file system.

That said, I don't know if anything would change, realistically, if we were to skip the resolution...

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you show me an example where resolving the parent of Path(__file__) is necessary?

Copy link
Member

Choose a reason for hiding this comment

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

I would imagine that probably not in the context of Python packaging. In general, it's probably useful when paths are processed by different programs with different access privileges.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed it to .resolve -- why not?

Copy link
Contributor

@wimglenn wimglenn Sep 8, 2023

Choose a reason for hiding this comment

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

Why not: because it should be no more complicated than necessary. I'm not convinced resolving the path is ever useful in this context.

No longer recommend methods
---------------------------

These methods rely on importing of code during the build process, or dynamically generating the version at build time. These methods are prone to errors and security issues, but you may encounter them in older code bases.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
These methods rely on importing of code during the build process, or dynamically generating the version at build time. These methods are prone to errors and security issues, but you may encounter them in older code bases.
These methods rely on importing of code during the build process and are prone to errors and security issues, but you may encounter them in older code bases.

I don't agree with this part of the sentence — it's perfectly fine to generate version during the build. setuptools-scm does just that — it makes a version by combining multiple sources — the last Git tag, distance to the last Git tag, current commit, current date, Git dirty flag, .git_archival.txt (for git archives and sdists).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure exactly what part you don't agree with -- but the idea is that running custom code at build time is not great -- which is different than using a declaration that invokes another tool that runs -- after all, something has to run to build a package :-) If that's not clear, then please suggest some more clear phrasing -- I'm at a loss.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've rephrased it bit -- maybe better?

Copy link
Member

Choose a reason for hiding this comment

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

Having reflected on this for a bit, I don't think this is right at all. Running custom code is fine, for as long as it's invoked through the standard interface. It's even documented in the official setuptools docs: https://setuptools.pypa.io/en/latest/build_meta.html#dynamic-build-dependencies-and-other-build-meta-tweaks.
Besides, it's quite mandatory to add some project-specific build code for building C-extensions. setuptools kinda expects that setup.py would be used for this. But personally, I've experimented with wrappers for it on the PEP 517 level and that went well.

Hatchling also has hooks for running Python code: https://hatch.pypa.io/1.1/plugins/build-hook/#custom

`commitizen <https://pypi.org/project/commitizen>`_,
`zest.releaser <https://pypi.org/project/zest.releaser>`_.
with open(os.path.join(mypackage_root_dir, 'VERSION')) as version_file:
version = version_file.read().strip()
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is just awful. It totally breaks zipapp-packaging. Recommend removal.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@ChrisBarker-NOAA ChrisBarker-NOAA Sep 7, 2023

Choose a reason for hiding this comment

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

I think this is a overall question -- this is already under "nearly extinct" -- I left it (and others) in because I thought it would be less controversial change (or less radical anyway) -- but I'm happy to simply remove the really old and crufty ones :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, they should be removed. The examples which import pkg_resources are cause deprecation warnings already.

@ChrisBarker-NOAA
Copy link
Contributor Author

I think I've addressed all the comments, one way or another.There's still a bit of philosophical question about to what extent this should document the current state of the practice, vs what is hopefully the future.

I suppose that part of resolving the question is the intended audience -- is this only for people starting from scratch with a new project? Or is it also for people maintaining older projects?

And another somewhat philisophical question -- packaging has moved in the direction of robust, best practices solutions that can handle all (most) cases. But this gets away from the KISS approach -- if someone is making a small package for internal use -- it's nice to have a simple way to simply solve the problem at hand, and I'd like to see that documented.

I'm not in a position to make these judgement calls.

@sinoroc
Copy link
Contributor

sinoroc commented Sep 8, 2023

I like that there is a healthy discussion.

Maybe this should be merged as a large overhaul of the whole page. Then work can begin to refine specific points.

source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
Note that the use of :file:`setup.cfg` is being depricated by setuptools -- new projects should use :file:`pyproject.toml`.

#. While the modern standard is to use declarative files (such as ``pyproject.toml``),
with older versions of setuptools, you may need to add a version-reading
Copy link
Contributor

Choose a reason for hiding this comment

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

setuptools will support setup.py forever. Maybe setup.cfg will disappear (hopefully not), but I am quite sure setup.py will stay for a very very long time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, yes the the declarative forms are still preferred -- and you can use setup.cfg or pyproject.toml alongside setup.py, yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reference to the deprecation?

Someone said that in this PR -- if that's not a done deal, we should remove it.

Copy link
Member

Choose a reason for hiding this comment

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

well, yes the the declarative forms are still preferred -- and you can use setup.cfg or pyproject.toml alongside setup.py, yes?

There's a few angles this can be viewed from:

  1. pyproject.toml's [build-system] section has nothing to do with setuptools directly as it's something that a build front-end uses. It can be used to make the builder select setuptools but that's the only link. So ideally, this file must always exist in modern projects. But the for non-frontend purposes, but backends, people may choose not to use it.
  2. Whenever setuptools is selected, it needs to get metadata from somewhere. It can get declarative metadata from:
    • different sections in pyproject.toml
    • setup.cfg
    • setup.py

When setup.py exists, the setuptools' build backend evaluates it, which in turn calls setuptools.setup() and that's what reads the declarative metadata in addition to anything generated prior to that call dynamically. In this case pyproject.toml or setup.cfg exist, it can load them and use as default values for settings not set via the setup() call.

When setup.cfg or pyproject.toml exist but not setup.py, then setuptools' backend emulates a setup.py file that only contains an import and a call to setup().

So yes, it is possible to have 1, 2 or 3 of these files in a project, depending on the preferences of the maintainainers.

Copy link
Member

Choose a reason for hiding this comment

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

setuptools will support setup.py forever. Maybe setup.cfg will disappear (hopefully not), but I am quite sure setup.py will stay for a very very long time.

Agreed. It'd be unfortunate and these were deprecated fully. There are cases when one would want to keep metadata in a dedicated file. In case of mass-maintenance of many projects, for example.

source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
source/guides/single-sourcing-package-version.rst Outdated Show resolved Hide resolved
@chrysle
Copy link
Contributor

chrysle commented Sep 19, 2023

Referring to #616 (comment), could you probably add a more complete example about the usage of importlib.metadata?

@pradyunsg pradyunsg marked this pull request as draft November 5, 2023 15:40
@pradyunsg
Copy link
Member

I've moved this to a draft status given the WIP in the title and merge conflicts, to avoid accident merges before the author or a maintainer is able to address those aspects.

@willingc
Copy link
Contributor

@ChrisBarker-NOAA Let's see if we can unblock this PR since the proposed content is an improvement over what exists. Next step would be to remove the conflict and take this out of WIP. For modernizing the guides, I think we should:

  • merge if significantly better than existing dated doc
  • make a note in the text if there is something that still requires further discussion by maintainers
  • make a note, if needed, at the top of the file explaining to the user that this has been modernized but is still under revision.


#. Place the value in a simple ``VERSION`` text file and have both
:file:`setup.py` and the project code read it.
.. Example using this technique: `warehouse <https://github.com/pypa/warehouse/blob/64ca42e42d5613c8339b3ec5e1cb7765c6b23083/warehouse/__about__.py>`_.
Copy link
Member

Choose a reason for hiding this comment

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

Why comment this out?

Nearly extinct methods
----------------------

These methods rely on importing of code during the build process, or dynamically generating the version string at build time with custom code. These methods are prone to errors and security issues, but you may encounter them in older code bases.
Copy link
Member

Choose a reason for hiding this comment

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

Per #1273 (comment), I don't think that there's inherent security issue implications here. This statement is not universally true.

We're talking about the build scripts, and they run dynamic stuff by definition. Even when it's abstracted away into external libs. A more realistic attack surface is the supply chain — poisoning the deps is much more effective and simpler, then checking in things into a project since they go through code reviews much more often, then the deps updates. This is why, in the articles related to publishing from GHA to PyPI, for example, we always emphasize that building must happen in a separate CI job that wouldn't be able to elevate privileges and publish packages using legitimate credentials.

I think, this paragraph should instead focus on the UX/DX/maintainability/elegance aspects of the approach.

Copy link
Member

Choose a reason for hiding this comment

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

And also on the fact that sometimes it's simply impossible to exec modules during the build, since that environment doesn't have runtime deps that might be imported from said module.

version = attr: my_package.__version__


Note that the use of :file:`setup.cfg` is being deprecated by setuptools -- new projects should use :file:`pyproject.toml`.
Copy link
Member

Choose a reason for hiding this comment

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

I think we established in the previous discussions that there's no merit in this statement right now. Let's remove the speculation...

Copy link
Contributor

Choose a reason for hiding this comment

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

Where was that established? As far as I know the plan from setuptools side is still to "eventually deprecate" setup.cfg, because it does not make sense to have two different declarative formats for the same purpose.

Ref: pypa/setuptools#3214

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps a good way to handle this is to put a note block in here and avoid deprecation comment here since it is not necessary for the reader.

In general, :file:`pyproject.toml` should be used for new projects.

Copy link
Member

Choose a reason for hiding this comment

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

Since it's a guide and not a tutorial, it probably shouldn't preach “the only proper way”, though. I'm personally a huge fan of using setup.cfg separately because it's a separate file. And I've seen some similar sentiment in that issue from other people. Perhaps, having a different file additionally would be a solution, but this is not a place to have that discussion.

I guess, my point is that I'd rather use an observation style rather than something imperative...

How about

Suggested change
Note that the use of :file:`setup.cfg` is being deprecated by setuptools -- new projects should use :file:`pyproject.toml`.
Note that many people prefer to stick with :file:`pyproject.toml` over :file:`setup.cfg`, while some still choose to keep using it regardless.

?

I think this would be pretty accurate.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t understand your proposed sentence. “stick with” means to keep using an old thing, no? pyproject.toml is the new thing, so you can’t “stick with” it.

Copy link
Member

Choose a reason for hiding this comment

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

You're right. Perhaps, I had a slightly different context in my head, thinking of a situation when there's a pyproject.toml, I want to separate it into a dedicated config (which would have to be setup.cfg) and somebody would want to keep using pyproject.toml.

Let's try rephrasing:

Suggested change
Note that the use of :file:`setup.cfg` is being deprecated by setuptools -- new projects should use :file:`pyproject.toml`.
Note that many people prefer :file:`pyproject.toml` over :file:`setup.cfg`, while some still choose to keep using the latter regardless.

@jeanas
Copy link
Contributor

jeanas commented Dec 22, 2023

@ChrisBarker-NOAA Let's see if we can unblock this PR since the proposed content is an improvement over what exists. Next step would be to remove the conflict and take this out of WIP. For modernizing the guides, I think we should:

  • merge if significantly better than existing dated doc

  • make a note in the text if there is something that still requires further discussion by maintainers

  • make a note, if needed, at the top of the file explaining to the user that this has been modernized but is still under revision.

+1 to @willingc's comment. While the concerns expressed around, e.g., to what extent setup.cfg is obsolete, are valid, the current content of the page is very out of date, so let's try to get it updated without blocking ourselves on larger questions.

@ChrisBarker-NOAA Do you still intend to work on this PR? If not, I'd be willing to submit a redux PR (possibly more focused).

@ChrisBarker-NOAA
Copy link
Contributor Author

Sorry I dropped the ball on this -- I"M more that happy for anyone else to pick it up -- and it's been another year, so maybe can be a bit simpler:

My advice: keep to documenting the current state of practice, and don't worry about focusing on the "best" way.

Most important is to not longer be documenting clearly out of data practices -- e.g. custom code to read the version string in a setup.py file.

@ChrisBarker-NOAA
Copy link
Contributor Author

ChrisBarker-NOAA commented Jul 23, 2024

I've gotten a bit confused about the state of this -- there is this old PR of mine, and this one: #1276

And a recent discussion on Discourse.

I think we do have consensus that keeping old, outdated docs published is a bad idea.

That PR, concludes with:

"I've considered all the comments here but still believe removing this page is the best way forward."

My thought:

Given that this page has been around a long time, simply removing it will create dead links, etc, so
I think slightly better is to replace this page with one that essentially says something along the lines of:

"""
It is best practice to specify the version number of your distribution in a single place, rather than trying to keep it in sync in more than one place.

Consult your build system's docs for how best to do that.
"""

And maybe a set of links to the most common build systems.

But we should do something in any case.

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.