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.

Sign up for GitHub

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

Jump to bottom

Exclude coverage for older Python version checks #4438

Closed
wants to merge 1 commit into from
Closed

Exclude coverage for older Python version checks #4438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions .coveragerc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ exclude_also =
# jaraco/skeleton#97
@overload
if TYPE_CHECKING:
# Coverage is only run on the latest Python version
# these will never be considered covered and we aim for 100% coverage
if sys\.version_info <
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically this matches <= (3, 12) but I think that's enough of an edge case, would require more complex regex and extra book-keeping when bumping the Python version, that it's not worth accounting for.

Copy link
Member

Choose a reason for hiding this comment

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

This file is managed primarily in skeleton and there's already an effort underway to implement coverage aggregation, which would obviate the need for this change.

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 better since then coverage would be correct in telling you that an environment-specific path was never taken.

Copy link
Member

Choose a reason for hiding this comment

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

I'm glad you agree. I'll assume we should close this request, then, and await/pursue the upstream change. If you feel differently, feel free to advocate for including this change and we can re-open.

6 changes: 3 additions & 3 deletions setuptools/_path.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
import sys
from typing import Union

if sys.version_info >= (3, 9):
StrPath = Union[str, os.PathLike[str]] # Same as _typeshed.StrPath
else:
if sys.version_info < (3, 9):
StrPath = Union[str, os.PathLike]
else:
StrPath = Union[str, os.PathLike[str]] # Same as _typeshed.StrPath
Comment on lines -6 to +8
Copy link
Member

Choose a reason for hiding this comment

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

I've historically written > (3, 9) (or the equivalent >= (3, 9)) because I like to put the canonical, preferred code ahead of the legacy code. I don't feel strongly about it and I feel less strongly about it when the compatibility logic is in a compat submodule.

My thinking is that if someone is skimming the code, if the legacy code is first, they'll need to read the whole block to get a modern understanding of what's going on, whereas if the preferred code is first, they can stop after reading the first block and just know that the rest is for older Python versions.

This concern illustrates is a case where an unless syntax would be so nice to have:

do:
    # preferred behavior
unless sys.version_info < (3, 9):
    # legacy behavior

Unfortunately, that's not a thing in Python, so the reader is forced to choose between the more elegant if clause or the more elegant preferred-first ordering.



def ensure_directory(path):
Expand Down
6 changes: 3 additions & 3 deletions setuptools/compat/py310.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
__all__ = ['tomllib']


if sys.version_info >= (3, 11):
import tomllib
else: # pragma: no cover
if sys.version_info < (3, 11):
from setuptools.extern import tomli as tomllib
else:
import tomllib
Loading