Added toml as extra

[Cielquan]

For using coverage with pyproject.toml you need to install coverage[toml].

For convenience and to reduce the dependency declaration lines for projects using pytest-cov and pyproject.toml I added the toml extra to pytest-cov as well.

Before (poetry examples):

[tool.poetry.dev-dependencies]
pytest-cov = "^2.8.1"
toml = "^0.10.1"

Or:

[tool.poetry.dev-dependencies]
pytest-cov = "^2.8.1"
coverage = {version = "^5", extras = ["toml"]}

Now:

[tool.poetry.dev-dependencies]
pytest-cov = {version = "^2.8.1", extras = ["toml"]}

[ionelmc]

Oooof. Well, here goes ...

[tool.poetry.dev-dependencies]
pytest-cov = {version = "^2.8.1", extras = ["toml"]}

If anyone would ask me I'd say that's a bad practice. An antipattern. Why should we even allow users use this footgun? We don't have a upper constraint on coverage because it would create lots of churn thus users will always be at risk of getting a broken coverage+pytest-cov combination in the future (eg: new coverage release). Thus it's always better to specify the coverage version you want in your project settings.

[Cielquan]

Oooof. Well, here goes ...

[tool.poetry.dev-dependencies]
pytest-cov = {version = "^2.8.1", extras = ["toml"]}

If anyone would ask me I'd say that's a bad practice. An antipattern. Why should we even allow users use this footgun? We don't have a upper constraint on coverage because it would create lots of churn thus users will always be at risk of getting a broken coverage+pytest-cov combination in the future (eg: new coverage release). Thus it's always better to specify the coverage version you want in your project settings.

I understand your concern and I think you are right for my initial proposal.

With the change proposed by @graingert I think this would fix the issue IF the pip issue mention above gets solved.

So there would still be no upper constraint either way and when the user wants to use pyproject.toml he/she can use the toml extra which is just a 'link' to coverage's toml extra. So even if coverage changes the extra to another lib it would work. It would also work when coverage moves toml to install_requires and removes the extra. Pip then gives a warning that the toml extra does not exist.

EDIT:
I would propose to put the PR on hold till the pip issue is fixed.
If you don't want to ad the feature at all please close the PR.

[hukkin]

Oooof. Well, here goes ...

[tool.poetry.dev-dependencies]
pytest-cov = {version = "^2.8.1", extras = ["toml"]}

If anyone would ask me I'd say that's a bad practice. An antipattern. Why should we even allow users use this footgun? We don't have a upper constraint on coverage because it would create lots of churn thus users will always be at risk of getting a broken coverage+pytest-cov combination in the future (eg: new coverage release). Thus it's always better to specify the coverage version you want in your project settings.

@ionelmc I'm not sure I agree with this. Isn't this what we have lockfiles for (in the case of Poetry, a poetry.lock)? That is, so we don't have to manually list and version lock each and every dependency of a dependency (recursively) in [tool.poetry.dev-dependencies]? The tooling does it for us.

@Cielquan Pip got it's dependency resolver in v20.3 and the issue you mentioned is closed 🥳 Maybe this would be doable now?

[Cielquan]

I accepted @graingert 's proposal for the chained toml dependency to coverage's one. I tested beforehand again and it worked with a fresh venv and pip 20.3 and pip 21.0.1.

Therefore I make it ready for review.

EDIT: Updated AUTHORS and CHANGELOG

[ionelmc]

Isn't the >=5 part redundant? It's already specified more precisely in install_requires.

[Cielquan]

Good point. I don't know.

[graingert]

fyi, pytest depends on toml directly

https://github.com/pytest-dev/pytest/blob/main/setup.cfg#L49

[hukkin]

fyi, pytest depends on toml directly

https://github.com/pytest-dev/pytest/blob/main/setup.cfg#L49

👍
I think it would be good for pytest-cov to depend on coverage[toml] directly (not via an extra) because

• toml is required anyways because of pytest so this doesnt add extra dependencies
• if coverage starts to restrict toml versions, the restriction will also apply to pytest-cov

[ionelmc]

I think we can depend on toml if it doesn't bring in any c extensions or other crazy stuff, just to be practical - I mean that's the idea of pytest-cov, to streamline and remove any warts if possible.

[graingert]

I think it would be good for pytest-cov to depend on coverage[toml] directly (not via an extra)

👍 I believe pip only warns if you depend on a version of coverage without the toml extra

[hukkin]

@graingert Yup correct. And because we require coverage>=5.2.1 and that version already has the toml extra, there is currently no compatible combination of package versions that would even issue that warning.

[Cielquan]

1. I think that pytest-cov depending on toml directly even though it is not a needed dependency of pytest-cov itself is not a good idea. Reason is simple. What if coverage changes from toml to e.g. tomlkit (which is IMO better) then we would be at the current point without this PR.

2. Even so pytest depends on toml directly who says that it will not change to another lib? (similar to 1. point)

3. I also think it is not good practise to depend on coverage[toml] from the beginning. When point 1 would come true and point 2 not, which would result in pytest depending on toml and coverage depending on anther toml lib, then someone who does not use pyproject.toml would have a useless dependency installed. I would rather highlight the available toml extra in the Readme/docs.

Those points have some ifs and point 2 and 3 are not that heavy impactful but those are just my two cents.

[hukkin]

I'd just add the install_requires=['coverage[toml]>=5.2.1']

When point 1 would come true and point 2 not, which would result in pytest depending on toml and coverage depending on anther toml lib, then someone who does not use pyproject.toml

Obviously we can't predict the future and prepare for everything, given infinite time, but if things change, we can change the dependencies.

As @ionelmc said:

I mean that's the idea of pytest-cov, to streamline and remove any warts if possible.

Remembering that you need an extra just because you want to have configuration in pyproject.toml to me sounds like one of these warts that pytest-cov could and should remove.

toml is a tiny (~1k LOC) pure python package that due to its popularity is probably installed anyways (i mean quite literally so due to pytest). The downside of it being installed and unused is a) minimal and b) theoretical (due to pytest).

[ionelmc]

I'd just add the install_requires=['coverage[toml]>=5.2.1']

That seems fine to me.

[Cielquan]

I'd just add the install_requires=['coverage[toml]>=5.2.1']

That seems fine to me.

Updated PR accordingly.