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

Reevaluate & fix types (now with stubs!) #119

Merged
merged 22 commits into from
Nov 23, 2023
Merged

Reevaluate & fix types (now with stubs!) #119

merged 22 commits into from
Nov 23, 2023

Conversation

gtronset
Copy link
Owner

@gtronset gtronset commented Nov 8, 2023

Overview

This is a comprehensive reevaluation of the type hints used by Filetote. With recent excellent efforts by Beets contributors (ex: beetbox/beets#4591), much more of beets has typing which means less guessing needs to occur and that stubs can be built out (recognizing the type hints added to beets are not yet released; stubs for it will be removed at a later date once possible).

This adds Type Hint Stubs for the following, which allows for the removal of --ignore-missing-imports from MyPy:

This also adds linting for .pyi (stub) files courtesy of flake8-pyi. As part of that, Flake8's config was extended for better compatibility with other linting/formatting (see: e5f94b3).

This also adds typeguard to run during pytests. Note: that functionality is disabled in Python 3.6 due to a bug in the last version of typeguard for it (see: agronholm/typeguard#168).

Finally, validation for the Filetote config has been added to ensure that at least the correct types are being set in the config itself.

Bonuses: deprecation warning for libraries used by confuse and mediafile are now ignored in Pytest; CI's running of pytest will be slightly faster.


A note on Paths

Beets has a long blog post on its strategy of handling path types; TL;DR they've opted to handle them everywhere as bytestrings (bytes). This plugin (and its predecessor copyartifacts) would use them interchangeably. This has been updated with the help of better type hints and the changes mentioned above.

While not documented explicitly, MoveOperation-related events expect bytes-type paths for their source and destination parameters. This was discovered by digging deeper here: https://github.com/beetbox/beets/blob/708a04d4a88eb955f261a21f0b1146c9e2ae0a55/beets/library.py#L915


Bugs fixed in this:


Future Changes

Additional refactoring is needed in the core handling of files to simplify and make it easier to understand. In particular relevance to this PR, there might be additional typing issues hidden in some of thee deeper parts that were not addressed here, due to the nature of how those methods are written.

@gtronset gtronset changed the title Add typehints & fix types Reevaluate & fix types (now with stubs!) Nov 16, 2023
@gtronset gtronset marked this pull request as ready for review November 19, 2023 01:21
@gtronset gtronset merged commit ff5e771 into main Nov 23, 2023
15 checks passed
@gtronset gtronset deleted the gt-str-byte-bug branch November 23, 2023 18:28
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.

None yet

1 participant