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
Changes from 2 commits
Commits
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
36 changes: 33 additions & 3 deletions mypy.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,35 @@
[mypy]
ignore_missing_imports = True
# required to support namespace packages
# https://github.com/python/mypy/issues/14057
# Is the project well-typed?
strict = False

# Early opt-in even when strict = False
warn_unused_ignores = True
warn_redundant_casts = True
enable_error_code = ignore-without-code

# Support namespace packages per https://github.com/python/mypy/issues/14057
explicit_package_bases = True

# Disable overload-overlap due to many false-positives
disable_error_code = overload-overlap

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.


# - distutils._modified has different errors on Python 3.8 [import-untyped], on Python 3.9+ [import-not-found]
Avasam marked this conversation as resolved.
Show resolved Hide resolved
# - 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.

# - 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-distutils._modified,jaraco.*,wheel.*]
ignore_missing_imports = True
Avasam marked this conversation as resolved.
Show resolved Hide resolved

# 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

...

Loading