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

Remove unnecessary code paths for 3.9+ (follow up on skeleton changes) #4718

Merged
merged 7 commits into from
Nov 4, 2024

Conversation

abravalheri
Copy link
Contributor

@abravalheri abravalheri commented Oct 30, 2024

Follow-up on e47994c.

Summary of changes

Closes

Pull Request Checklist

@abravalheri
Copy link
Contributor Author

abravalheri commented Oct 30, 2024

I have added the news fragments as feature, although they are removals...
For 3.9+, this should not matter much.

This approach seems to be consistent with the existing changelog that classifies these changes in requires-python as features.

I can rename the news fragment files if necessary.

@abravalheri abravalheri changed the title Remove unnecessary code paths for 3.9+ Remove unnecessary code paths for 3.9+ (follow up on skeleton changes) Oct 30, 2024
@abravalheri abravalheri marked this pull request as ready for review October 30, 2024 17:41
@Avasam
Copy link
Contributor

Avasam commented Oct 30, 2024

  • You can move setuptools._path.StrPath out of the TYPE_CHECKING check !
  • functools.lru_cache(maxsize=None) can be replaced by functools.cache *
  • Callable, Generator, Iterator, Iterable, Sequence, Mapping, MutableMapping should be imported from collections.abc instead of typing *
  • Type, Dict, List, Tuple should be replaced by their builtin variant *
  • Optional should be replaced by Union for consistency

*UP035 (which includes https://github.com/PyCQA/flake8-pyi/blob/main/ERRORCODES.md#Y022 ) should catches those, but it looks like the include = ["pyproject.toml"] workaround broke it ! :O

I think include = ["pyproject.toml"] should be replaced by target-version = "py39" # Target the oldest supported Python version . The comment and link are still correct.

Edit: because it should be extend = "pyproject.toml", not include...

@Avasam
Copy link
Contributor

Avasam commented Oct 30, 2024

Additional tidbit I found: setuptools.package_index._splituser has a comment about Python 3.8, but it would be nice if it linked to python/cpython#80072

@abravalheri
Copy link
Contributor Author

Thank you @Avasam, I implemented the suggestions in:

I am leaving the fix to ruff.toml to skeleton, we can see the feedback there and then decide what to do.

@abravalheri abravalheri merged commit bf582cb into pypa:main Nov 4, 2024
22 of 24 checks passed
@abravalheri abravalheri deleted the update-3.9 branch November 4, 2024 15:49
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.

3 participants