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

Revamp good practices #10206

Merged
merged 8 commits into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/en/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@
"tox": ("https://tox.wiki/en/stable", None),
"virtualenv": ("https://virtualenv.pypa.io/en/stable", None),
"setuptools": ("https://setuptools.pypa.io/en/stable", None),
"packaging": ("https://packaging.python.org/en/latest", None),
}


Expand Down
168 changes: 79 additions & 89 deletions doc/en/explanation/goodpractices.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,41 +12,26 @@ For development, we recommend you use :mod:`venv` for virtual environments and
as well as the ``pytest`` package itself.
This ensures your code and dependencies are isolated from your system Python installation.

Next, place a ``pyproject.toml`` file in the root of your package:
Create a ``pyproject.toml`` file in the root of your repository as described in
:doc:`packaging:tutorials/packaging-projects`.
The first few lines should look like this:

.. code-block:: toml

[build-system]
requires = ["setuptools >= 42"]
build-backend = "setuptools.build_meta"

and a ``setup.cfg`` file containing your package's metadata with the following minimum content:

.. code-block:: ini
requires = ["hatchling"]
Copy link
Member

Choose a reason for hiding this comment

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

I personally prefer that we use setuptools in our examples, because it is more traditional; using alternatives might give the impression that pytest recommends/only works with hatchling, or support requests on how to do the same with poetry, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, maybe we can also use tabs like in the packaging tutorial?

I don’t recommend setuptools as it’s:

  • slow
  • messy (leaves a .egg-info folder lying around in your workspace, emits a bunch of unnecessary log messages during build)
  • hard to configure (a bunch of legacy config files like MANIFEST.in, and misleadingly named config keys)
  • no longer the first choice on the packaging tutorial

but of course I can leave it if you prefer. I’d still get rid of setup.cfg since setuptools these days supports PEP 621. Unfortunately configuring it with more than just basic metadata is labelled as “beta”, e.g. this:

[tool.setuptools.packages.find]
where = ["src"]  # ["."] by default

Copy link
Member

@nicoddemus nicoddemus Aug 19, 2022

Choose a reason for hiding this comment

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

no longer the first choice on the packaging tutorial

Ahh thanks for pointing that out, didn't know that has changed. If the packaging tutorial recommends hatchling as first choice, I think we should follow suit then.

Copy link
Contributor Author

@flying-sheep flying-sheep Aug 20, 2022

Choose a reason for hiding this comment

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

In case you would like some context, I followed that discussion, and the reasoning went something like that:

  1. we don’t want to promote any standards following build backend above all others
  2. so we use tabs
  3. but since the first (auto-selected) tab is kind of an endorsement, we need something there that is good for the vast majority of people, i.e. ideal for pure python packages
  4. because of the above mentioned drawbacks of setuptools, the first tab will be hatchling, because it provides a better experience for people creating a pure python package than setuptools, has a plugin to use setuptool-scm for versioning (hatch-vcs)

Hatchling doesn’t have a sufficient amount of plugins or example code yet to allow a highly custom build like numpy or numba or so, but for anything with purely static build configuration, it’s a very good choice.

build-backend = "hatchling.build"

[metadata]
name = PACKAGENAME

[options]
packages = find:
name = "PACKAGENAME"

where ``PACKAGENAME`` is the name of your package.

.. note::

If your pip version is older than ``21.3``, you'll also need a ``setup.py`` file:

.. code-block:: python

from setuptools import setup

setup()

You can then install your package in "editable" mode by running from the same directory:

.. code-block:: bash

pip install -e .
pip install -e .

which lets you change your source code (both tests and application) and rerun tests at will.

Expand Down Expand Up @@ -89,11 +74,11 @@ to keep tests separate from actual application code (often a good idea):
.. code-block:: text

pyproject.toml
setup.cfg
mypkg/
__init__.py
app.py
view.py
src/
mypkg/
__init__.py
app.py
view.py
tests/
test_app.py
test_view.py
Expand All @@ -112,72 +97,28 @@ This has the following benefits:
See :ref:`pytest vs python -m pytest` for more information about the difference between calling ``pytest`` and
``python -m pytest``.

Note that this scheme has a drawback if you are using ``prepend`` :ref:`import mode <import-modes>`
(which is the default): your test files must have **unique names**, because
``pytest`` will import them as *top-level* modules since there are no packages
to derive a full package name from. In other words, the test files in the example above will
be imported as ``test_app`` and ``test_view`` top-level modules by adding ``tests/`` to
``sys.path``.
For new projects, we recommend to use ``importlib`` :ref:`import mode <import-modes>`
(see which-import-mode_ for a detailed explanation).
To this end, add the following to your ``pyproject.toml``:

If you need to have test modules with the same name, you might add ``__init__.py`` files to your
``tests`` folder and subfolders, changing them to packages:

.. code-block:: text
.. code-block:: toml

pyproject.toml
setup.cfg
mypkg/
...
tests/
__init__.py
foo/
__init__.py
test_view.py
bar/
__init__.py
test_view.py
[tool.pytest.ini_options]
addopts = [
"--import-mode=importlib",
]

Now pytest will load the modules as ``tests.foo.test_view`` and ``tests.bar.test_view``, allowing
you to have modules with the same name. But now this introduces a subtle problem: in order to load
the test modules from the ``tests`` directory, pytest prepends the root of the repository to
``sys.path``, which adds the side-effect that now ``mypkg`` is also importable.
.. _src-layout:

This is problematic if you are using a tool like `tox`_ to test your package in a virtual environment,
because you want to test the *installed* version of your package, not the local code from the repository.
Generally, but especially if you use the default import mode ``prepend``,
it is **strongly** suggested to use a ``src`` layout.
Here, your application root package resides in a sub-directory of your root,
i.e. ``src/mypkg/`` instead of ``mypkg``.

.. _`src-layout`:
This layout prevents a lot of common pitfalls and has many benefits,
which are better explained in this excellent `blog post`_ by Ionel Cristian Mărieș.

In this situation, it is **strongly** suggested to use a ``src`` layout where application root package resides in a
sub-directory of your root:

.. code-block:: text

pyproject.toml
setup.cfg
src/
mypkg/
__init__.py
app.py
view.py
tests/
__init__.py
foo/
__init__.py
test_view.py
bar/
__init__.py
test_view.py


This layout prevents a lot of common pitfalls and has many benefits, which are better explained in this excellent
`blog post by Ionel Cristian Mărieș <https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure>`_.

.. note::
The ``--import-mode=importlib`` option (see :ref:`import-modes`) does not have
any of the drawbacks above because ``sys.path`` is not changed when importing
test modules, so users that run into this issue are strongly encouraged to try it.

The ``src`` directory layout is still strongly recommended however.
.. _blog post: https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure>


Tests as part of application code
flying-sheep marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -190,8 +131,7 @@ want to distribute them along with your application:
.. code-block:: text

pyproject.toml
setup.cfg
mypkg/
[src/]mypkg/
__init__.py
app.py
view.py
Expand Down Expand Up @@ -253,6 +193,56 @@ Note that this layout also works in conjunction with the ``src`` layout mentione
much less surprising.


.. _which-import-mode:

Choosing an import mode
^^^^^^^^^^^^^^^^^^^^^^^

For historical reasons, pytest defaults to the ``prepend`` :ref:`import mode <import-modes>`
instead of the ``importlib`` import mode we recommend for new projects.
The reason lies in the way the ``prepend`` mode works:

Since there are no packages to derive a full package name from,
``pytest`` will import your test files as *top-level* modules.
The test files in the first example (:ref:`src layout <src-layout>`) would be imported as
``test_app`` and ``test_view`` top-level modules by adding ``tests/`` to ``sys.path``.

This results in a drawback compared to the import mode ``importlib``:
your test files must have **unique names**.

If you need to have test modules with the same name,
as a workaround you might add ``__init__.py`` files to your ``tests`` folder and subfolders,
changing them to packages:

.. code-block:: text

pyproject.toml
mypkg/
...
tests/
__init__.py
foo/
__init__.py
test_view.py
bar/
__init__.py
test_view.py

Now pytest will load the modules as ``tests.foo.test_view`` and ``tests.bar.test_view``,
allowing you to have modules with the same name.
But now this introduces a subtle problem:
in order to load the test modules from the ``tests`` directory,
pytest prepends the root of the repository to ``sys.path``,
which adds the side-effect that now ``mypkg`` is also importable.

This is problematic if you are using a tool like tox_ to test your package in a virtual environment,
because you want to test the *installed* version of your package,
not the local code from the repository.

The ``importlib`` import mode does not have any of the drawbacks above,
because ``sys.path`` is not changed when importing test modules.


.. _`buildout`: http://www.buildout.org/en/latest/

.. _`use tox`:
Expand Down
14 changes: 2 additions & 12 deletions doc/en/how-to/fixtures.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1762,8 +1762,6 @@ Given the tests file structure is:
::

tests/
__init__.py

conftest.py
# content of tests/conftest.py
import pytest
Expand All @@ -1778,8 +1776,6 @@ Given the tests file structure is:
assert username == 'username'

subfolder/
__init__.py

conftest.py
# content of tests/subfolder/conftest.py
import pytest
Expand All @@ -1788,8 +1784,8 @@ Given the tests file structure is:
def username(username):
return 'overridden-' + username

test_something.py
# content of tests/subfolder/test_something.py
test_something_else.py
# content of tests/subfolder/test_something_else.py
def test_username(username):
assert username == 'overridden-username'

Expand All @@ -1805,8 +1801,6 @@ Given the tests file structure is:
::

tests/
__init__.py

conftest.py
# content of tests/conftest.py
import pytest
Expand Down Expand Up @@ -1848,8 +1842,6 @@ Given the tests file structure is:
::

tests/
__init__.py

conftest.py
# content of tests/conftest.py
import pytest
Expand Down Expand Up @@ -1886,8 +1878,6 @@ Given the tests file structure is:
::

tests/
__init__.py

conftest.py
# content of tests/conftest.py
import pytest
Expand Down