-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Ensure PlatformDirs is valid superclass type for mypy AND not an abstract class for other checkers #295
Ensure PlatformDirs is valid superclass type for mypy AND not an abstract class for other checkers #295
Conversation
The doc test doesn't like Thankfully |
|
||
def _set_platform_dir_class() -> type[PlatformDirsABC]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that if python/mypy#10962 is fixed, this should become
def _set_platform_dir_class() -> type[PlatformDirsABC]: | |
def _set_platform_dir_class() -> type[_Result | Android]: |
But that'd require duplicating the from platformdirs.android import Android
import in the TYPE_CHECKING
block to keep it a lazy import. And it brings no benefit at the current time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [platformdirs](https://github.com/platformdirs/platformdirs) | `==4.2.2` -> `==4.3.2` | [![age](https://developer.mend.io/api/mc/badges/age/pypi/platformdirs/4.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/platformdirs/4.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/platformdirs/4.2.2/4.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/platformdirs/4.2.2/4.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>platformdirs/platformdirs (platformdirs)</summary> ### [`v4.3.2`](https://github.com/tox-dev/platformdirs/releases/tag/4.3.2) [Compare Source](https://github.com/platformdirs/platformdirs/compare/4.3.1...4.3.2) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed - Use uv as installer by [@​gaborbernat](https://github.com/gaborbernat) in [https://github.com/tox-dev/platformdirs/pull/300](https://github.com/tox-dev/platformdirs/pull/300) - Fix multi-path returned from `_path` methods on MacOS by [@​matthewhughes934](https://github.com/matthewhughes934) in [https://github.com/tox-dev/platformdirs/pull/299](https://github.com/tox-dev/platformdirs/pull/299) #### New Contributors - [@​matthewhughes934](https://github.com/matthewhughes934) made their first contribution in [https://github.com/tox-dev/platformdirs/pull/299](https://github.com/tox-dev/platformdirs/pull/299) **Full Changelog**: tox-dev/platformdirs@4.3.1...4.3.2 ### [`v4.3.1`](https://github.com/tox-dev/platformdirs/releases/tag/4.3.1) [Compare Source](https://github.com/platformdirs/platformdirs/compare/4.3.0...4.3.1) <!-- Release notes generated using configuration in .github/release.yml at main --> **Full Changelog**: tox-dev/platformdirs@4.3.0...4.3.1 ### [`v4.3.0`](https://github.com/tox-dev/platformdirs/releases/tag/4.3.0) [Compare Source](https://github.com/platformdirs/platformdirs/compare/4.2.2...4.3.0) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed - Speed up Hatch installation by [@​ofek](https://github.com/ofek) in [https://github.com/tox-dev/platformdirs/pull/282](https://github.com/tox-dev/platformdirs/pull/282) - Test with Python 3.13 by [@​edgarrmondragon](https://github.com/edgarrmondragon) in [https://github.com/tox-dev/platformdirs/pull/289](https://github.com/tox-dev/platformdirs/pull/289) - Test with latest PyPy by [@​edgarrmondragon](https://github.com/edgarrmondragon) in [https://github.com/tox-dev/platformdirs/pull/290](https://github.com/tox-dev/platformdirs/pull/290) - Use `include-hidden-files: true` to upload coverage artifacts by [@​edgarrmondragon](https://github.com/edgarrmondragon) in [https://github.com/tox-dev/platformdirs/pull/298](https://github.com/tox-dev/platformdirs/pull/298) - Ensure PlatformDirs is valid superclass type for mypy AND not an abstract class for other checkers by [@​Avasam](https://github.com/Avasam) in [https://github.com/tox-dev/platformdirs/pull/295](https://github.com/tox-dev/platformdirs/pull/295) #### New Contributors - [@​edgarrmondragon](https://github.com/edgarrmondragon) made their first contribution in [https://github.com/tox-dev/platformdirs/pull/289](https://github.com/tox-dev/platformdirs/pull/289) - [@​Avasam](https://github.com/Avasam) made their first contribution in [https://github.com/tox-dev/platformdirs/pull/295](https://github.com/tox-dev/platformdirs/pull/295) **Full Changelog**: tox-dev/platformdirs@4.2.2...4.3.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/googleapis/sdk-platform-java). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->
This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [platformdirs](https://github.com/platformdirs/platformdirs) | `==4.2.2` -> `==4.3.2` | [![age](https://developer.mend.io/api/mc/badges/age/pypi/platformdirs/4.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/platformdirs/4.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/platformdirs/4.2.2/4.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/platformdirs/4.2.2/4.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>platformdirs/platformdirs (platformdirs)</summary> ### [`v4.3.2`](https://github.com/tox-dev/platformdirs/releases/tag/4.3.2) [Compare Source](https://github.com/platformdirs/platformdirs/compare/4.3.1...4.3.2) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed - Use uv as installer by [@​gaborbernat](https://github.com/gaborbernat) in [https://github.com/tox-dev/platformdirs/pull/300](https://github.com/tox-dev/platformdirs/pull/300) - Fix multi-path returned from `_path` methods on MacOS by [@​matthewhughes934](https://github.com/matthewhughes934) in [https://github.com/tox-dev/platformdirs/pull/299](https://github.com/tox-dev/platformdirs/pull/299) #### New Contributors - [@​matthewhughes934](https://github.com/matthewhughes934) made their first contribution in [https://github.com/tox-dev/platformdirs/pull/299](https://github.com/tox-dev/platformdirs/pull/299) **Full Changelog**: tox-dev/platformdirs@4.3.1...4.3.2 ### [`v4.3.1`](https://github.com/tox-dev/platformdirs/releases/tag/4.3.1) [Compare Source](https://github.com/platformdirs/platformdirs/compare/4.3.0...4.3.1) <!-- Release notes generated using configuration in .github/release.yml at main --> **Full Changelog**: tox-dev/platformdirs@4.3.0...4.3.1 ### [`v4.3.0`](https://github.com/tox-dev/platformdirs/releases/tag/4.3.0) [Compare Source](https://github.com/platformdirs/platformdirs/compare/4.2.2...4.3.0) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed - Speed up Hatch installation by [@​ofek](https://github.com/ofek) in [https://github.com/tox-dev/platformdirs/pull/282](https://github.com/tox-dev/platformdirs/pull/282) - Test with Python 3.13 by [@​edgarrmondragon](https://github.com/edgarrmondragon) in [https://github.com/tox-dev/platformdirs/pull/289](https://github.com/tox-dev/platformdirs/pull/289) - Test with latest PyPy by [@​edgarrmondragon](https://github.com/edgarrmondragon) in [https://github.com/tox-dev/platformdirs/pull/290](https://github.com/tox-dev/platformdirs/pull/290) - Use `include-hidden-files: true` to upload coverage artifacts by [@​edgarrmondragon](https://github.com/edgarrmondragon) in [https://github.com/tox-dev/platformdirs/pull/298](https://github.com/tox-dev/platformdirs/pull/298) - Ensure PlatformDirs is valid superclass type for mypy AND not an abstract class for other checkers by [@​Avasam](https://github.com/Avasam) in [https://github.com/tox-dev/platformdirs/pull/295](https://github.com/tox-dev/platformdirs/pull/295) #### New Contributors - [@​edgarrmondragon](https://github.com/edgarrmondragon) made their first contribution in [https://github.com/tox-dev/platformdirs/pull/289](https://github.com/tox-dev/platformdirs/pull/289) - [@​Avasam](https://github.com/Avasam) made their first contribution in [https://github.com/tox-dev/platformdirs/pull/295](https://github.com/tox-dev/platformdirs/pull/295) **Full Changelog**: tox-dev/platformdirs@4.2.2...4.3.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/googleapis/sdk-platform-java). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->
This was spotted in https://github.com/jaraco/jaraco.abode/blob/8843f360ee5d9bc1afb1bdb3119157addb4aaf06/jaraco/abode/config.py#L4C7-L4C19 / jaraco/skeleton#136 (comment)
Trying to subclass
PlatformDirs
(and by extension,AppDirs
) results in the following error with mypy:Variable "platformdirs.PlatformDirs" is not valid as a type
This is a known issue: python/mypy#10962
Thankfully it can be worked around, which this PR does, as well as adding a static test for it.
Then another issue is revealed, since
PlatformDirs
is typed as being an abstract class, it shouldn't be allowed to be instantiated ! So I can't just do:I'm honnestly a bit dumbofunded by that one. It's a perfect unfixeable storm.
I can't type
PlatformDirs
as aTypeAlias
oftype[Result] | type[Android]
because we're right back to square one.And without https://peps.python.org/pep-0738/ I can't just alias
PlatformDirs
toResult
otherwiseif PlatformDirs is Android
will always be raised as an issue by type_checkers. Same if I makePlatformDirs
a fake subclass ofPlatformDirsABC
in theTYPE_CHECKING
block.I think the false positive on
if PlatformDirs is Android
is probably much better than a false positive onclass MySubclass(PlatformDirs): ...
So I'll go for that.