Add meaningful tests to new-layout

[fabianhauser]

This PR adds a more meaningful template-test to the poetry new standard layout. The new test checks whether the module-version and the pyproject.toml-version are equal.

Pull Request Check List

Resolves: -

• Added tests for changed code.
  • I'm not sure how to test this change properly - suggestions are welcome.
• Updated documentation for changed code.

[das-g]

[Full disclosure: I was the one to suggest upstreaming this after @fabianhauser proposed this change in a poetry-generated project.]

Diff LGTM, change is sensible. 👍

Ship it!

[abn]

Thank you for taking the time to contribute to the poetry. 👍

The package.__version__ value is not currently managed by poetry outside of new, this means bumping the version would not affect this value. The new command itself is there more to allow an easy initialisation of boilerplate.

This change adds an otherwise unused dev dependency toml to the new project. Considering that this does not change the status quo significantly enough to warrant this addition, I am inclined to not approve this change.

[das-g]

The package.__version__ value is not currently managed by poetry outside of new, this means bumping the version would not affect this value. The new command itself is there more to allow an easy initialisation of boilerplate.

Yeah, we're aware of that. Actually, that's the reason for this PR, as it's thus the respective newly generated project's developers' responsibility to keep the versions in project.toml and package.__version__ in sync. And they would probably have to do that manually, lest they introduce their own automation. The suggested example test, if they choose to keep it, supports them in doing so.

In contrast, the example test currently generated with the template introduces more duplication without any meaningful benefit.

If the example test was the test suggested by this PR, some projects might choose to keep and adopt it while adding their own tests, while other projects might choose to delete it. In contrast, with the example test currently generated with the template, the only reasonable action after poetry new is to delete it (at least as soon as one doesn't need it anymore as a template on how and where to put one's own tests), as it's more an obstacle than any help.

An example test that's worth keeping as an actual test if the generated projects' developers so choose is IMO a good thing.

This change adds an otherwise unused dev dependency toml to the new project.

That is indeed the downside of this PR. If this PR was to be reconsidered, would you prefer the entry in project.toml to be accompanied by a comment that the entry can and shall be deleted if the example test is removed? (Not yet sure how to achieve that. Maybe a comment towards that end in the test code would also do the trick.)

[finswimmer]

Hello,

I would suggest a complete different approach: The version of the package shouldn't be hardcoded within the code. Instead importlib.metadata (importlib_metada for python < 3.8) should be used, to read the version provided by the package during installation.

Option 1: Completely removal of __version__ from __init__.py
Option 2: Add this instead to __init__.py:

try:
    import importlib.metadata as importlib_metadata
except ModuleNotFoundError:
    import importlib_metadata

__version__ = importlib_metadata.version(__name__)

And add this to the pyproject.toml:

[tool.poetry.dependencies]
...
importlib-metadata = {version = "^1.0", python = "<3.8"}

The most clean version would be, to check if the project should support python <3.8 at all (or vice versa). If not the additional stuff for this should not be included.

fin swimmer

[aviramha]

Hello,

I would suggest a complete different approach: The version of the package shouldn't be hardcoded within the code. Instead importlib.metadata (importlib_metada for python < 3.8) should be used, to read the version provided by the package during installation.

Option 1: Completely removal of __version__ from __init__.py
Option 2: Add this instead to __init__.py:

try:
    import importlib.metadata as importlib_metadata
except ModuleNotFoundError:
    import importlib_metadata

__version__ = importlib_metadata.version(__name__)

And add this to the pyproject.toml:

[tool.poetry.dependencies]
...
importlib-metadata = {version = "^1.0", python = "<3.8"}

The most clean version would be, to check if the project should support python <3.8 at all (or vice versa). If not the additional stuff for this should not be included.

fin swimmer

What about the case of a microservice using poetry (not an actual python package)?
Version doesn't exist because it's never installed..

[finswimmer]

Hello @aviramha,

What about the case of a microservice using poetry (not an actual python package)?

IMO in that case you should:

a) distribute your pyproject.toml together with to code and just read the version from there
b) have a custom build script, that put your files together you want to distribute. And this script could read the version from pyproject.toml and put it wherever you like.

fin swimmer

[logangrado]

Hello,

I would suggest a complete different approach: The version of the package shouldn't be hardcoded within the code. Instead importlib.metadata (importlib_metada for python < 3.8) should be used, to read the version provided by the package during installation.

Option 1: Completely removal of __version__ from __init__.py

If this were the case, would it still be possible to read the version from within python (either from within the package, or when imported by another package)?

Option 2: Add this instead to __init__.py:

try:
    import importlib.metadata as importlib_metadata
except ModuleNotFoundError:
    import importlib_metadata

__version__ = importlib_metadata.version(__name__)

And add this to the pyproject.toml:

[tool.poetry.dependencies]
...
importlib-metadata = {version = "^1.0", python = "<3.8"}

The most clean version would be, to check if the project should support python <3.8 at all (or vice versa). If not the additional stuff for this should not be included.

fin swimmer

I feel like there's got to be a better solution than adding additional dependencies importlib_metadata, it just feels messy. Don't have a better answer, but it feels messy.

Anyway, if this is the standard moving forward, will this be added to poetry new?

[das-g]

I feel like there's got to be a better solution than adding additional dependencies importlib_metadata, it just feels messy. Don't have a better answer, but it feels messy.

That'd be only an additional dependency for Python < 3.8. For Python ≥ 3.8, importlib.metadata is part of the Python standard library. And it's probably worth noting, that it'd "only" be an additional main dependency: It's already present in newly generated projects as an indirect dev dependency because pytest directly and indirectly (via pluggy) depends on it. Off course, as a main dependency, it's also propagated to all software that will depend on the newly generated project, so this is still worth being carefully evaluated.

[mawillcockson]

try:
    import importlib.metadata as importlib_metadata
except ModuleNotFoundError:
    import importlib_metadata

__version__ = importlib_metadata.version(__name__)

Currently, python/mypy#1153 blocks this from being a great solution, as it currently produces:

temp/__init__.py:4: error: Name 'importlib_metadata' already defined (by an import)
        import importlib_metadata
        ^
Found 1 error in 1 file (checked 2 source files)

Since there is no timeline on a fix, as mentioned in a comment in the linked issue thread, maybe a solution from bhrutledge/mypy-importlib-metadata could be useful.

[corymosiman12]

Hello,

I would suggest a complete different approach: The version of the package shouldn't be hardcoded within the code. Instead importlib.metadata (importlib_metada for python < 3.8) should be used, to read the version provided by the package during installation.

Option 1: Completely removal of __version__ from __init__.py
Option 2: Add this instead to __init__.py:

try:
    import importlib.metadata as importlib_metadata
except ModuleNotFoundError:
    import importlib_metadata

__version__ = importlib_metadata.version(__name__)

And add this to the pyproject.toml:

[tool.poetry.dependencies]
...
importlib-metadata = {version = "^1.0", python = "<3.8"}

The most clean version would be, to check if the project should support python <3.8 at all (or vice versa). If not the additional stuff for this should not be included.

fin swimmer

I'm testing this out using semantic versioning. I do the following:

poetry version 0.1.0-alpha.0

# [package-name] 0.1.0-alpha.0 is returned

When I then specify that in the version test, I get the following:

    def test_version():
>       assert __version__ == '0.1.0-alpha.0'
E       AssertionError: assert '0.1.0a0' == '0.1.0-alpha.0'
E         - 0.1.0-alpha.0
E         + 0.1.0a0

Seems somehow 0.1.0-alpha.0 is getting translated to 0.1.0a0 via the importlib_metadata.version() function. Any thoughts?

• Python 3.7.4
• Poetry version 1.0.10

[finswimmer]

Seems somehow 0.1.0-alpha.0 is getting translated to 0.1.0a0 via the importlib_metadata.version() function. Any thoughts?

This happens when the package is build, because poetry allows you to follow SemVer specifications in the pyproject.toml, but PEP440 says we have to normalize versions that contains hyphen to that you see here.

[tueda]

 __version__ = importlib_metadata.version(__name__)

The use of __name__ may not work when the author starts to use a non-standard layout for the project (tool.poetry.packages.include).

[mawillcockson]

 __version__ = importlib_metadata.version(__name__)

The use of __name__ may not work when the author starts to use a non-standard layout for the project (tool.poetry.packages.include).

Could you give an example of where this may not work?

Does tool.poetry.packages.include refer to the way that non-standard project layouts are specified when using poetry?

If so, I believe that the __name__ variable is the appropriate place to source the name of the distribution from, since that should be set to the uniquely-identifying name of the module. I am not an expert on Python packaging, though.

Here's a script that creates an example project layout with the package folder in a src/ directory at the root of the project, with the project layout looking like:

temp
├── pyproject.toml
├── src
│   └── temp
│       ├── __init__.py
│       ├── __main__.py
│       └── main.py
└── tests
    ├── __init__.py
    └── test_version.py

#!/bin/sh
set -eux

mkdir -p temp/src/temp/
mkdir -p temp/tests/

cd temp

cat << EOF > pyproject.toml
[tool.poetry]
name = "temp"
version = "0.1.0"
description = ""
authors = []
packages = [
    { include = "temp", from = "src" },
]

[tool.poetry.dependencies]
python = "^3.7"
importlib-metadata = {version = "latest", python = "<3.8"}

[tool.poetry.dev-dependencies]
pytest = "^5.2"

[tool.poetry.scripts]
temp = "temp.main:main"

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"
EOF

cat << EOF > src/temp/__init__.py
"""
This file causes python to treat the folder it's in as a package.

It's also used to find the version string.
"""
import sys

if sys.version_info >= (3, 8):
    from importlib.metadata import version as metadata_version
else:
    from importlib_metadata import version as metadata_version

__version__ = str(metadata_version(__name__))
EOF

cat << EOF > src/temp/__main__.py
"""
This is the file run when this project is run as a module:

python -m module_name
"""
from .main import main

if __name__ == "__main__":
    main()
EOF

cat << EOF > src/temp/main.py
"""
The main file for when the project is run
"""
from argparse import ArgumentParser

from . import __version__


def main() -> None:
    parser = ArgumentParser()
    parser.add_argument("--version", action="version", version=__version__)
    parser.parse_args()


if __name__ == "__main__":
    main()
EOF

touch tests/__init__.py

cat << EOF > tests/test_version.py
"""
tests for the version string only
"""
import re

from temp import __version__

simple_semver_re = re.compile(r"^(\\d+\\.){2}\\d+$")


def test_version() -> None:
    "make sure the version is a string that matches semver format"
    assert isinstance(
        __version__, str
    ), f"__version__ must be a str, not '{type(__version__)}'"
    assert simple_semver_re.match(
        __version__
    ), f"'{__version__}' is not in simple semver.org format"
EOF

poetry install
poetry run temp --version
poetry run python -m temp --version
poetry run pytest

[tueda]

@mawillcockson I agree that __name__ should be the fully-qualified name of the module in the system, and probably the same as the distribution name in most cases. But, in principle, I think this could be different (see name of this setup.py; you can find many example-pkg-* in PyPI). Here is an example of a Poetry project:

oh-util-is-taken-in-pypi-so-i-have-to-change-the-distname
├── pyproject.toml
├── util
│   ├── __init__.py
│   ├── __main__.py
│   └── main.py
└── tests
    ├── __init__.py
    └── test_version.py

[tool.poetry]
name = "oh-util-is-taken-in-pypi-so-i-have-to-change-the-distname"
packages = [
    { include = "util" },
]

(Though this may be not good from the viewpoint of avoiding possible conflicts.)

[mawillcockson]

@mawillcockson I agree that __name__ should be the fully-qualified name of the module in the system, and probably the same as the distribution name in most cases. But, in principle, I think this could be different (see name of this setup.py; you can find many example-pkg-* in PyPI). Here is an example of a Poetry project:

oh-util-is-taken-in-pypi-so-i-have-to-change-the-distname
├── pyproject.toml
├── util
│   ├── __init__.py
│   ├── __main__.py
│   └── main.py
└── tests
    ├── __init__.py
    └── test_version.py

[tool.poetry]
name = "oh-util-is-taken-in-pypi-so-i-have-to-change-the-distname"
packages = [
    { include = "util" },
]

(Though this may be not good from the viewpoint of avoiding possible conflicts.)

That's a very good point. Finding the distribution name programmatically is somewhat involved, though it is possible.

If the project is being generated through poetry new, the name given could be hardcoded in __init__.py, but then that's back to the problem of including metadata in the project source, outside of pyproject.toml.

If anyone reading this runs into this, in order to use the example I proposed, the distribution name of the project must match the package name: the name of the module in which the __init__.py file that is looking up the version is located, must be the same as the name = in pyproject.toml.

oh-util-is-taken-in-pypi-so-i-have-to-change-the-distname/__init__.py

---

[tool.poetry]
name = "oh-util-is-taken-in-pypi-so-i-have-to-change-the-distname"

The name of the command that the package includes can still be anything:

[tool.poetry.scripts]
different-name = "oh_util_is_taken_in_pypi_so_i_have_to_change_the_distname.main:main"

[tiangolo]

I love Poetry for everything. 🚀

But I've faced similar issues like those described here.

So, I just made a plugin (available with Poetry 1.2.0a1 and above) to read the version from an __init__.py file:

# __init__.py

__version__ = "0.1.0"

or from a git tag, set with a GitHub release or with:

$ git tag 0.1.0

I thought this could be useful for others here too as this seems to be the place of the main conversation.

https://github.com/tiangolo/poetry-version-plugin

[hadrien]

What about using pkg_resources from setuptools? IIUC works for python >= 2.7

import pkg_resources

__version__ = pkg_resources.get_distribution(package_name).version

-> https://setuptools.readthedocs.io/en/latest/pkg_resources.html#distribution-attributes

[hadrien]

What about using pkg_resources from setuptools? IIUC works for python >= 2.7

import pkg_resources

__version__ = pkg_resources.get_distribution(package_name).version

-> https://setuptools.readthedocs.io/en/latest/pkg_resources.html#distribution-attributes

FYI, we (@dialoguemd) do this ^.

We use pyproject.toml as the source of truth for version.

It is updated automatically at CI whenever something is merged on main branch by using python semantic-release.

[adam-grant-hendry]

Hello,

I would suggest a complete different approach: The version of the package shouldn't be hardcoded within the code. Instead importlib.metadata (importlib_metada for python < 3.8) should be used, to read the version provided by the package during installation.

Option 1: Completely removal of __version__ from __init__.py Option 2: Add this instead to __init__.py:

try:
    import importlib.metadata as importlib_metadata
except ModuleNotFoundError:
    import importlib_metadata

__version__ = importlib_metadata.version(__name__)

And add this to the pyproject.toml:

[tool.poetry.dependencies]
...
importlib-metadata = {version = "^1.0", python = "<3.8"}

The most clean version would be, to check if the project should support python <3.8 at all (or vice versa). If not the additional stuff for this should not be included.

fin swimmer

Does this also work for packages installed in development mode (develop = true) or new packages being developed in rapid-development that have yet to be published?

[animesh-workplace]

Looks good to me

[adborden]

I tried adding this code manually to my project, but after bumping the version the test no longer passed. To be sure, I tried with a fresh example:

$ poetry install
$ poetry run poetry new ../example-template
$ cd ../example-template/
$ poetry install
$ poetry run pytest # pass
$ poetry version patch
$ poetry install
Installing dependencies from lock file

No dependencies to install or update

Installing the current project: example-template (0.1.1)
$ poetry run pytest
=========================================================== test session starts ===========================================================
platform darwin -- Python 3.8.11, pytest-5.4.3, py-1.11.0, pluggy-0.13.1
rootdir: /Users/aaronborden/projects/devops/example-template
collected 1 item

tests/test_example_template.py F                                                                                                    [100%]

================================================================ FAILURES =================================================================
______________________________________________________________ test_version _______________________________________________________________

    def test_version():
        pyproject_file = Path(__file__).parents[1] / "pyproject.toml"
        pyproject = toml.load(pyproject_file)
        pyproject_version = pyproject["tool"]["poetry"]["version"]

>       assert module_version == pyproject_version
E       AssertionError: assert '0.1.0' == '0.1.1'
E         - 0.1.1
E         ?     ^
E         + 0.1.0
E         ?     ^

tests/test_example_template.py:13: AssertionError
========================================================= short test summary info =========================================================
FAILED tests/test_example_template.py::test_version - AssertionError: assert '0.1.0' == '0.1.1'
============================================================ 1 failed in 0.07s ============================================================
$ cat tests/test_example_template.py
from pathlib import Path

import toml

from example_template import __version__ as module_version


def test_version():
    pyproject_file = Path(__file__).parents[1] / "pyproject.toml"
    pyproject = toml.load(pyproject_file)
    pyproject_version = pyproject["tool"]["poetry"]["version"]

    assert module_version == pyproject_version

The only work around I found was to manually remove the poetry virtual env.

[abn]

Poetry no longer adds any test or any dependencies when creating a new project via poetry new. See default layout at https://github.com/python-poetry/poetry/blob/master/src/poetry/layouts/layout.py.

Closing as this is no longer relevant.

[abn]

Thank you @fabianhauser for getting this together, we appreciate the contribution.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.