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

[pyproject.toml: part 3] Add means to apply configuration from pyproject.toml #3067

Merged
merged 17 commits into from
Mar 18, 2022

Conversation

abravalheri
Copy link
Contributor

@abravalheri abravalheri commented Feb 1, 2022

This is part of #2970, split into several stacked PRs as requested in #2970 (review)
Follows #3066

This change targets adding a standardised project configuration format as initially proposed by PEP 621 (with the possibility of modification/evolution according to follow-up PEPs).

For the configuration fields not covered by the standards, the approach proposed in ini2toml is adopted.

This operation relies on vendorised external libraries tomli and validate-pyproject.

Summary of changes

  • Implement read_configuration from pyproject.toml - 3eb62ac
  • Expand dynamic entry_points from pyproject.toml - 112e8a9
  • Add means of applying config read from pyproject.toml to dist - 303791a
  • Add the apply_configuration API to setuptools.config.setupcfg - 43e3436
  • Test pyproject.toml config has the same effect as setup.cfg - c6907cb

Please see the commit message of each individual commit for more information about the changes.

Pull Request Checklist

@abravalheri abravalheri requested a review from webknjaz February 1, 2022 12:34
@abravalheri abravalheri changed the title [pyproject.toml: part 3] Add means to apply configuration from pyprojec.toml [pyproject.toml: part 4] Add means to apply configuration from pyprojec.toml Feb 1, 2022
@abravalheri abravalheri marked this pull request as ready for review February 3, 2022 10:22
@abravalheri abravalheri force-pushed the dist-config-from-pyproject branch 2 times, most recently from 72fe662 to 5ef62fc Compare February 8, 2022 11:54
@webknjaz
Copy link
Member

webknjaz commented Feb 9, 2022

@abravalheri FYI the PR title has a typo (missing "t" in "pyproject"), I can't fix it myself.

@abravalheri abravalheri changed the title [pyproject.toml: part 4] Add means to apply configuration from pyprojec.toml [pyproject.toml: part 4] Add means to apply configuration from pyproject.toml Feb 9, 2022

@pytest.mark.parametrize("url", EXAMPLE_URLS)
@pytest.mark.filterwarnings("ignore")
def test_apply_pyproject_equivalent_to_setupcfg(url, monkeypatch, tmp_path):
Copy link
Member

Choose a reason for hiding this comment

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

Am I reading this right that this relies on network I/O? Maybe this could be marked as such?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes @webknjaz, this test will try to download a bunch of real world examples... It is possible to cache in the CI, but I haven't implemented that yet.

Is your suggestion adding a simple @pytest.mark.uses_network, or something more complex?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(when running the tests locally, the files will be cached)

return path


def download(url, dest):
Copy link
Member

Choose a reason for hiding this comment

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

It's probably worth implementing a global cache and avoiding making fixture download part of testing. It should be possible to request cache download or --offline via a pytest CLI flag that could be implemented via a pytest hook callback.

Copy link
Member

Choose a reason for hiding this comment

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

Some of related ideas have been suggested in the past @ #2607

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 think for the time being I will add a uses_network mark, so these tests can be skipped with -m "not uses_network" if necessary.

As a follow up, my intention is to add caching to the CI (locally the caching will already take place).

An --offline CLI flag would require a more comprehensive effort by checking from all existing tests which ones use the network, mark them with the uses_network mark and then doing a trick in conftest.py similar to the _skip_integration... So I would also leave it as "future work".

.gitignore Outdated Show resolved Hide resolved
value = re.sub(r"^Project-URL: Homepage,.*$", "", value, flags=re.M)
# May be missing in original (relying on default) but backfilled in the TOML
value = re.sub(r"^Description-Content-Type:.*$", "", value, flags=re.M)
# ini2toml can automatically convert `tests_require` to `testing` extra
Copy link
Member

Choose a reason for hiding this comment

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

Why not "tests"? I would expect that "testing" contains helpers for the end-users to test their project using the given package while "tests" would correspond to its own internal test requirements.

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 ended up going with testing because this seems to be popular in the community... I have the impression that this approach was influenced by the fact that tox docs explicitly mention testing.

For example setuptools and virtualenv use testing.

@abravalheri abravalheri force-pushed the dist-config-from-pyproject branch from 5ef62fc to bfb2ff2 Compare February 9, 2022 19:44
@abravalheri
Copy link
Contributor Author

Thank you very much for the review @webknjaz.

Regarding the offline tests, I tried to mark the tests here so they can be skipped. I am afraid implementing --offline in pytest will be a much bigger scope and involve changes in existing tests, so I am deferring that for the future.

@abravalheri abravalheri force-pushed the dist-config-from-pyproject branch 2 times, most recently from 9dcd88b to 5ecdb5b Compare February 10, 2022 20:18
@abravalheri abravalheri force-pushed the dist-config-from-pyproject branch from 5ecdb5b to c2b80aa Compare February 11, 2022 19:30
@abravalheri abravalheri changed the title [pyproject.toml: part 4] Add means to apply configuration from pyproject.toml [pyproject.toml: part 3] Add means to apply configuration from pyproject.toml Feb 11, 2022
@abravalheri abravalheri force-pushed the dist-config-from-pyproject branch from c2b80aa to 69ddad0 Compare February 18, 2022 22:40
@abravalheri abravalheri force-pushed the dist-config-from-pyproject branch from 69ddad0 to 19b6ce2 Compare February 19, 2022 19:44
abravalheri added a commit to abravalheri/setuptools that referenced this pull request Feb 19, 2022
This change targets adding a standardised project configuration format
as initially proposed by PEP 621 (with the possibility of
modification/evolution according to follow-up PEPs).

For the configuration fields not covered by the standards, the approach
proposed in `ini2toml` is adopted.

This operation relies on vendorised external libraries `tomli` and
`validate-pyproject`.
abravalheri added a commit to abravalheri/setuptools that referenced this pull request Feb 20, 2022
This change targets adding a standardised project configuration format
as initially proposed by PEP 621 (with the possibility of
modification/evolution according to follow-up PEPs).

For the configuration fields not covered by the standards, the approach
proposed in `ini2toml` is adopted.

This operation relies on vendorised external libraries `tomli` and
`validate-pyproject`.
abravalheri added a commit that referenced this pull request Feb 20, 2022
This change targets adding a standardised project configuration format
as initially proposed by PEP 621 (with the possibility of
modification/evolution according to follow-up PEPs).

For the configuration fields not covered by the standards, the approach
proposed in `ini2toml` is adopted.

This operation relies on vendorised external libraries `tomli` and
`validate-pyproject`.
abravalheri and others added 17 commits March 5, 2022 14:30
This is the first step towards making setuptools understand
`pyproject.toml` as a configuration file.

The implementation deliberately allows splitting the act of loading the
configuration from a file in 2 stages: the reading of the file itself
and the expansion of directives (and other derived information).
The user might specify dynamic `entry-points` via a `file:`
directive (a similar feature for `setup.cfg` is documented in
[declarative config]).

The changes introduced here add the ability to expand them
when reading the configuration from `pyproject.toml`.

[declarative config]: https://setuptools.pypa.io/en/latest/userguide/declarative_config.html
There is frequent an opinion in the community that
`include_package_data=True` is a better default
(and a quality of life improvement).

Since we are migrating to a new configuration file, this change can
be implemented in a backward compatible way
(to avoid breaking existing packages):

- Config from `setup.cfg` defaults to `include_package_data=False`
- Config from `pyproject.toml` defaults to `include_package_data=True`

This also takes advantage that `ini2toml` (the provided library for
automatic conversion between `setup.cfg` and `pyproject.toml`) will
backfill `include_package_data=False` when the field is missing.
Since the Distrubition and DistributionMetadata classes are modeled
after (an old version of) core metadata, it is necessary to add a
translation layer between them and the configuration read from
pyproject.toml
The apply_configuration is implemented in a way that it is consistent
for both pyproject.toml and setup.cfg
According to PEP 621, the backend should fill-in the content-type if the
`readme` field is passed as a string. The value is derived from the
extension of the file (an error should be raised when that is not
possible).

Previously all READMEs were wrongly assumed rst.
This error was reported in:

https://discuss.python.org/t/help-testing-experimental-features-in-setuptools/13821/4
@abravalheri abravalheri force-pushed the dist-config-from-pyproject branch from 979596a to 0497954 Compare March 5, 2022 14:41
Base automatically changed from add-vendored-deps to support-pyproject-aggregate March 18, 2022 10:08
@abravalheri abravalheri merged commit 2cfcf0e into support-pyproject-aggregate Mar 18, 2022
@abravalheri abravalheri deleted the dist-config-from-pyproject branch March 18, 2022 10:09
Copy link
Member

Choose a reason for hiding this comment

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

@abravalheri so I rediscovered this file by accident and wanted to say that I saw a corner case with setup.cfg behaving differently from pyproject.toml in the case of multiple package roots (in that pyproject.toml works while setup.cfg doesn't): https://github.com/ansible/ansible/pull/81440/files#diff-fa602a8a75dc9dcc92261bac5f533c2a85e34fcceaff63b3a3a81d9acde2fc52R40-R42.

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.

2 participants