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

Update mypy.ini from setuptools #136

Merged
merged 5 commits into from
Aug 12, 2024
Merged

Update mypy.ini from setuptools #136

merged 5 commits into from
Aug 12, 2024

Conversation

Avasam
Copy link
Contributor

@Avasam Avasam commented Jul 30, 2024

Includes the latest changes from setuptools (including pypa/setuptools#4526) that I believe are applicable to all skeleton-based projects.

Avoiding first and second person.
Copy link
Owner

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

Thanks for the contrib. I appreciate you working to converge the divergence in Setuptools.

This change pushes pretty hard against my goal for zero config. Ideally, tools should enact best practices and downstream users should have to tune very few knobs. This change proposes to 7x the amount of boilerplate (by lines).

I understand that some of these settings are to work toward best practices that had to be opt-in for compatibility reasons (strict = False), but others feel like cruft.

Let's see if we can trim this down to the most essential settings.

mypy.ini Outdated
Comment on lines 16 to 22
exclude = (?x)(
# upstream
^build/
| ^.tox/

# local
)
Copy link
Owner

@jaraco jaraco Aug 8, 2024

Choose a reason for hiding this comment

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

I really feel like this should be unnecessary. I've never had problems with it. What problem does it address? Can we eliminate it?

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'll try to give you concrete examples when I have time.

But off the top of my head, it can lead to issues with mypy picking up python files copied or created in these folders.
For example mypy might think there's a duplicate module and not able to differentiate it. Or give errors for python modules not part of the source code. Even when it doesn't error it'll speed up the run by not scanning useless files.

I don't remember if you can tell mypy to follow a .gitignore, but that's irrelevant given you prefer not using one.

I know the skeleton uses tox. I don't remember if it has anything that should generate a "build" folder.

Copy link
Owner

Choose a reason for hiding this comment

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

I've decided to remove this for now. Until there's a good justification for it, I'd rather it not be here.

mypy.ini Outdated Show resolved Hide resolved
mypy.ini Outdated
)

# - distutils._modified has different errors on Python 3.8 [import-untyped], on Python 3.9+ [import-not-found]
# - All jaraco modules are still untyped
Copy link
Owner

Choose a reason for hiding this comment

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

Some (many?) jaraco modules are now typed, so this no longer applies.

Copy link
Contributor

Choose a reason for hiding this comment

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

Many aren't typed strictly as #98 is still open, but yeah, it no longer applies

Copy link
Contributor Author

@Avasam Avasam Aug 9, 2024

Choose a reason for hiding this comment

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

This will improve with time anyway (both in terms of inaccurate/incomplete typing, and being marked as py.typed), I don't mind not including it in the base recommendation for the sake of trimming it.
But my annecdotal experience with setuptools' specific dependencies, most weren't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah
I guess we could adopt a tool like https://github.com/jellezijlstra/autotyping to speed up the process, wdyt @Avasam?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, sorry.
To speedrun* the process. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I guess we could adopt a tool like jellezijlstra/autotyping to speed up the process, wdyt @Avasam?

Never tried it, but such a tool from Jelle is worth trying.

Worth mentioning that https://github.com/google/pytype (which typeshed tests agains) claims to be able to generate stub files from inference (it works from the bottom-up, unlike most type-checkers), which can then be merged back into source code.

In any case, I'd prefer for a package to be confident in its typing before publicising itself as py.typed.

mypy.ini Outdated

# - distutils._modified has different errors on Python 3.8 [import-untyped], on Python 3.9+ [import-not-found]
# - All jaraco modules are still untyped
# - wheel appears to be untyped
Copy link
Owner

Choose a reason for hiding this comment

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

If wheel is still untyped, this comment should point to a ticket tracking getting it typed.

Copy link
Contributor

@bswck bswck Aug 8, 2024

Choose a reason for hiding this comment

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

It is inherently wrong to say "untyped". I opened the code and there are a lot of type hints.
What is missing is the py.typed marker and some functions are truly untyped--and that's what needs to be addressed.

Copy link
Contributor Author

@Avasam Avasam Aug 8, 2024

Choose a reason for hiding this comment

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

@bswck I copied the comment from setuptools. But yes "marked as py.typed" is the verbage I usually prefer. mypy will say "found module but no type hints or library stubs".

As for it not being marked as typed, see pypa/wheel#610 (comment) which is relevant.

Copy link
Contributor Author

@Avasam Avasam Aug 10, 2024

Choose a reason for hiding this comment

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

Am I right in understanding that importing wheel is a setuptools-specific concern ?
https://github.com/pypa/wheel/pull/620/files

Sounds like the package is becomming only a CLI tool and the functionality split across packaging and setuptools.

Copy link
Owner

Choose a reason for hiding this comment

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

Am I right in understanding that importing wheel is a setuptools-specific concern ?

Yes. That's my understanding as well, so probably should be omitted from here.

mypy.ini Outdated
Comment on lines 30 to 35
# Even when excluding a module, import issues can show up due to following import
# https://github.com/python/mypy/issues/11936#issuecomment-1466764006
# Add your excluded modules that still produce import errors here
# [mypy-a_module.*]
# follow_imports = silent
# silent => ignore errors when following imports
Copy link
Owner

Choose a reason for hiding this comment

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

Probably this kind of guidance should go in the skeleton docs (gh-pages branch) with maybe a one-line link at the top of the file.

Copy link
Contributor Author

@Avasam Avasam Aug 9, 2024

Choose a reason for hiding this comment

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

Sure! Not sure exactly where this goes in https://github.com/jaraco/skeleton/blob/gh-pages/index.md though, maybe a new section about mypy's config? But it sounds like that can be done in parallel, so I'll remove this section from this file now.

Suggested change
# Even when excluding a module, import issues can show up due to following import
# https://github.com/python/mypy/issues/11936#issuecomment-1466764006
# Add your excluded modules that still produce import errors here
# [mypy-a_module.*]
# follow_imports = silent
# silent => ignore errors when following imports

Copy link
Owner

Choose a reason for hiding this comment

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

I thought maybe there was already a relevant section, but I don't see one. I think it should go under Packaging Conventions after Running Tests, something like Type Checking

Packaging Conventions

Running Tests

...

Type Checking

...

Continuous Integration

...

mypy.ini Outdated Show resolved Hide resolved
@Avasam Avasam requested a review from jaraco August 12, 2024 13:05
mypy.ini Outdated Show resolved Hide resolved
@jaraco
Copy link
Owner

jaraco commented Aug 22, 2024

This change has led to a lot of work. I spent the full day today working through the failures introduced in my projects, and I'm only about 1/3 through the failures. It's a lot of work to identify upstream issues and associate the exceptions with those. See pmxbot/pmxbot, importlib_metadata, jaraco.abode, jaraco.develop for examples of the changes necessary. I started to fatigue and just lump exceptions without upstream issues (jaraco/keyring@5cd3828).

There have been a couple of failures also due to issues with improper merges, but most of the issues are relating to the mypy changes.

This work is not good use of my time.

@Avasam Would you be willing to work through the remaining failing projects (excluding coherent.*) and devise PRs to fix those? Or can you propose some other way to deal with the fallout from this in a less labor-intensive way?

@Avasam
Copy link
Contributor Author

Avasam commented Aug 22, 2024

Is there a specific kind of issue you're hitting more often ? Maybe there's some config we can add to be even more lenient on a project's first pass.

For instance, it seems a lot of your issues are related to missing stubs / untyped dependencies. Maybe we should add

# Enable import-untyped if you'd like to be notified about using untyped dependencies or missing stubs packages
# You can leave it disabled if your package isn't publicly typed / `py.typed`
disable_error_code = import-untyped

until the ecosystem is further along. Namely the jaraco packages. (that's why I originally ignored that issue from jaraco.* in setuptools)

If a package doesn't yet intend to promote itself as py.typed, I don't think it needs to care about the import-untyped error as much (it matters once your typed API interacts with one of your dependencies' types). Whilst still having some basic type-checking benefits.

I also don't think that running around after untyped dependencies is the best use of your time. Especially not this early in type-checking adoption. Packages getting typed will be a slow trickle.

If you get any issue with stubs in typeshed, I can prioritize your blockers there.
win32ctypes, win32cred and pywintypes in keyring all come from types-pywin32

https://github.com/jaraco/jaraco.abode/blob/8843f360ee5d9bc1afb1bdb3119157addb4aaf06/jaraco/abode/config.py#L4C7-L4C19 is a mypy issue (python/mypy#10962) although that can be worked around in platformdirs itself. Edit: I opened tox-dev/platformdirs#295


I can go through https://jaraco.com/projects/ and create PRs if you'd like. Although my next two days are going to be quite occupied. This weekend I'd have time for.

clrpackages pushed a commit to clearlinux-pkgs/pypi-importlib_resources that referenced this pull request Aug 22, 2024
….4.3 to version 6.4.4

Avasam (2):
      Add Protocols, remove @overload, from `.coveragerc` `exclude_also` (jaraco/skeleton#135)
      Loosen restrictions on mypy (jaraco/skeleton#136)

Dimitri Papadopoulos Orfanos (1):
      Update to the latest ruff version (jaraco/skeleton#137)

Jason R. Coombs (7):
      Split the test dependencies into four classes (test, cover, type, check). (jaraco/skeleton#139)
      Add upstream and local sections for 'type' extra, since many projects will have 'types-*' dependencies.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      Finalize
@jaraco jaraco mentioned this pull request Aug 25, 2024
53 tasks
@jaraco
Copy link
Owner

jaraco commented Aug 25, 2024

Rather that continue the conversation here, I've filed #143 to track the effort.

clrpackages pushed a commit to clearlinux-pkgs/pypi-jaraco.collections that referenced this pull request Aug 30, 2024
…0.1 to version 5.1.0

Anderson Bravalheri (1):
      Add `--fix`  flag to ruff pre-commit hook for automatic suggestion of fixes (jaraco/skeleton#140)

Avasam (4):
      Add Protocols, remove @overload, from `.coveragerc` `exclude_also` (jaraco/skeleton#135)
      Loosen restrictions on mypy (jaraco/skeleton#136)
      Pass mypy and link issues
      Fully typed `RangeMap` and avoid complete iterations to find matches (#16)

Bartosz Sławecki (1):
      Move project metadata to `pyproject.toml` (jaraco/skeleton#122)

Dimitri Papadopoulos Orfanos (3):
      "preserve" does not require preview any more (jaraco/skeleton#133)
      Enforce ruff/Perflint rule PERF401 (jaraco/skeleton#132)
      Update to the latest ruff version (jaraco/skeleton#137)

Jason R. Coombs (14):
      Pin against pytest 8.1.x due to pytest-dev/pytest#12194.
      Migrated config to pyproject.toml using jaraco.develop.migrate-config and ini2toml.
      Allow macos on Python 3.8 to fail as GitHub CI has dropped support.
      Move project.urls to appear in the order that ini2toml generates it. Remove project.scripts.
      Revert "Allow macos on Python 3.8 to fail as GitHub CI has dropped support."
      Rename extras to align with core metadata spec.
      Prefer "Source" to "Homepage" for the repository label.
      Exclude pytest-ruff (and thus ruff), which cannot build on cygwin.
      Re-enable preview, this time not for one specific feature, but for all features in preview.
      Split the test dependencies into four classes (test, cover, type, check). (jaraco/skeleton#139)
      Add upstream and local sections for 'type' extra, since many projects will have 'types-*' dependencies.
      Disable mypy for now. Ref jaraco/skeleton#143
      Finalize
      Remove unsupported attribution. See jaraco/skeleton#144.
clrpackages pushed a commit to clearlinux-pkgs/pypi-zipp that referenced this pull request Aug 30, 2024
…n 3.20.1

Anderson Bravalheri (1):
      Add `--fix`  flag to ruff pre-commit hook for automatic suggestion of fixes (jaraco/skeleton#140)

Avasam (2):
      Add Protocols, remove @overload, from `.coveragerc` `exclude_also` (jaraco/skeleton#135)
      Loosen restrictions on mypy (jaraco/skeleton#136)

Dimitri Papadopoulos Orfanos (1):
      Update to the latest ruff version (jaraco/skeleton#137)

Jason R. Coombs (19):
      Uncomment link to the docs.
      Split the test dependencies into four classes (test, cover, type, check). (jaraco/skeleton#139)
      Add upstream and local sections for 'type' extra, since many projects will have 'types-*' dependencies.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      Add reference to development methodology.
      Disable mypy for now. Ref jaraco/skeleton#143
      Add two tests capturing the expectation for unsupported names.
      Adjust the expectation in test_malformed_paths to expect empty paths ignored and .. to be a path segment.
      Removed SanitizedNames.
      Address infinite loop when zipfile begins with more than one leading slash.
      Refine expectation that paths with leading slashes are simply not visible.
      Invent DirtyZipInfo to create an unsanitized zipfile with backslashes.
      Add news fragment.
      Mark unused code as uncovered.
      Prefer simpler path.rstrip to consolidate checks for empty or only paths.
      Add TODO to consolidate this behavior in CPython.
      Finalize
clrpackages pushed a commit to clearlinux-pkgs/pypi-setuptools that referenced this pull request Aug 30, 2024
…version 74.0.0

Anderson Bravalheri (11):
      Remove outdated import error check for warnings
      Satisfy ruff import format
      Add `--fix`  flag to ruff pre-commit hook for automatic suggestion of fixes (jaraco/skeleton#140)
      Remove unnecessary conditional on TYPE_CHECKING
      Test setuptools own sdist does not include tox files
      Prune .tox directory
      Remove custom manifest_maker.prune_file_list and rely on default implementation in sdist
      Automatically exclude top-level .tox|.nox|.ven from sdist
      Test against false positve matches
      Account for windows path separators in tests
      Add missing news fragment for PR 4603

Avasam (18):
      Add Protocols, remove @overload, from `.coveragerc` `exclude_also` (jaraco/skeleton#135)
      Loosen restrictions on mypy (jaraco/skeleton#136)
      Raise `TypeError` in `easy_install.CommandSpec.from_param`
      Type-check on all Python versions
      Pin Ruff to a lower bound rather than pinning pytest-ruff to an upper-bound
      Move static-checkers-only dependencies into their dedicated extras
      Avoid issues with  _build_ext being Any (#4599)
      `pkg_resources`: use `_typeshed.importlib.LoaderProtocol` (#4597)
      Make get_ext_filename typesafe (#4592)
      `pkg_resources` fully type all collection attributes (#4598)
      Made setuptools.package_index.Credential a NamedTuple (#4585)
      Initial pyright config (#4192)
      Type str/repr dunders (#4582)
      Type all get/set dunders
      Type all comparison/operators dunders (#4583)
      Type context manager dunders (#4581)
      Link to issues in mypy.ini for non py.typed dependencies (#4561)
      Reraise sensible errors from auto_chmod

Cal Jacobson (3):
      handle failures to find a user home directory
      Update distutils/dist.py
      format

Chris Barker (1):
      change VERSION to __version__ in the example (#4590)

Christoph Reiter (1):
      Remove unused wininst-*.exe files

Dimitri Papadopoulos (9):
      Apply ruff/tryceratops rule TRY300
      Apply ruff/tryceratops rule TRY301
      Enforce ruff/tryceratops rules (TRY)
      Enforce ruff/flake8-2020 rule (YTT301)
      Apply ruff/Perflint rule PERF102
      Apply ruff/Perflint rule PERF402
      Apply ruff/Perflint rule PERF403
      Enforce ruff/Perflint rules (PERF)
      Add `# local` to ignore conflicts with upstrea

Dimitri Papadopoulos Orfanos (1):
      Update to the latest ruff version (jaraco/skeleton#137)

Jason R. Coombs (17):
      Split the test dependencies into four classes (test, cover, type, check). (jaraco/skeleton#139)
      Add upstream and local sections for 'type' extra, since many projects will have 'types-*' dependencies.
      Disable mypy for now. Ref jaraco/skeleton#143
      Simply suppress the exception.
      Sort imports
      Define the variable in one place.
      Extract logic around the mappings into a function to compute the spec.
      Tweak docstring
      Remove legacy msvc compiler modules.
      Remove associated tests
      Remove monkeypatching of _msvccompiler.
      Remove associated tests
      Apply downstream VS 2017 support.
      Apply error message from downstream.
      Add news fragment.
      Add news fragment.
      Bump version: 73.0.1 → 74.0.0

Kagami Sascha Rosylight (8):
      Use arm64 MSVC on arm64 Windows
      Check MSVC arm64 variant on arm64 host (#4555)
      conditionally construct PLAT_TO_VCVARS
      function-ify
      nit
      python supports comparison chains
      apply feedback
      restore the lost comment
clrpackages pushed a commit to clearlinux-pkgs/pypi-inflect that referenced this pull request Sep 10, 2024
…ion 7.4.0

Anderson Bravalheri (1):
      Add `--fix`  flag to ruff pre-commit hook for automatic suggestion of fixes (jaraco/skeleton#140)

Avasam (2):
      Add Protocols, remove @overload, from `.coveragerc` `exclude_also` (jaraco/skeleton#135)
      Loosen restrictions on mypy (jaraco/skeleton#136)

Dimitri Papadopoulos Orfanos (3):
      "preserve" does not require preview any more (jaraco/skeleton#133)
      Enforce ruff/Perflint rule PERF401 (jaraco/skeleton#132)
      Update to the latest ruff version (jaraco/skeleton#137)

Jason R. Coombs (14):
      Exclude pytest-ruff (and thus ruff), which cannot build on cygwin.
      Re-enable preview, this time not for one specific feature, but for all features in preview.
      Split the test dependencies into four classes (test, cover, type, check). (jaraco/skeleton#139)
      Add upstream and local sections for 'type' extra, since many projects will have 'types-*' dependencies.
      🧎‍♀️ Genuflect to the types.
      Move comment
      Construct the _gend_sing dictionary statically.
      Disable mypy for now. Ref jaraco/skeleton#143
      Move overload-overlap disablement to its own line for easier diffs and simpler relevant comments.
      Add a degenerate nitpick_ignore for downstream consumers. Add a 'local' comment to delineate where the skeleton ends and the downstream begins.
      Add support for linking usernames.
      Include the trailing slash in disable_error_code(overload-overlap), also required for clean diffs.
      Add news fragment.
      Finalize

Josh Stephenson (1):
      Clarify that inflect is for English.

Mitchel Humpherys (2):
      Handle a single apostrophe more gracefully
      Add a test for a single-character apostrophe string
clrpackages pushed a commit to clearlinux-pkgs/pypi-importlib_metadata that referenced this pull request Sep 16, 2024
…4.0 to version 8.5.0

Anderson Bravalheri (1):
      Add `--fix`  flag to ruff pre-commit hook for automatic suggestion of fixes (jaraco/skeleton#140)

Avasam (4):
      Add Protocols, remove @overload, from `.coveragerc` `exclude_also` (jaraco/skeleton#135)
      Loosen restrictions on mypy (jaraco/skeleton#136)
      Pass mypy and link issues
      Update tests/_path.py with jaraco.path 3.7.2

Daniel Hollas (3):
      Defer import of zipp
      Defer json import
      Defer platform import

Dimitri Papadopoulos Orfanos (1):
      Update to the latest ruff version (jaraco/skeleton#137)

Jason R. Coombs (24):
      Split the test dependencies into four classes (test, cover, type, check). (jaraco/skeleton#139)
      👹 Feed the hobgoblins (delint).
      Remove superfluous parentheses.
      Rely on zipp overlay for zipfile.Path.
      Add upstream and local sections for 'type' extra, since many projects will have 'types-*' dependencies.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      🧎‍♀️ Genuflect to the types.
      Add reference to development methodology.
      Disable mypy for now. Ref jaraco/skeleton#143
      Move overload-overlap disablement to its own line for easier diffs and simpler relevant comments.
      Add a degenerate nitpick_ignore for downstream consumers. Add a 'local' comment to delineate where the skeleton ends and the downstream begins.
      Add support for linking usernames.
      Include the trailing slash in disable_error_code(overload-overlap), also required for clean diffs.
      Add comment to protect the deferred import.
      Add news fragment.
      Revert "Defer platform import"
      Add comment to protect the deferred import.
      Add news fragment.
      Back out changes to tests._path
      Finalize
This pull request was closed.
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