plugins: add more type annotations #2634
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In `PyFilePlugin.__init__()`, the `filename` must be wrapped in a list before passing it to `importlib.util.spec_from_file_location()`, whose `submodule_search_locations` argument is expected to be a list of `str`. That +1/-1 patch should be released as a hotfix for 8.0.x, but the rest of the type changes and doc tweaks can wait for 8.1.
Exirel
approved these changes
Oct 18, 2024
| # core | ||
| self._regexes = regexes | ||
| self._plugin_name = plugin | ||
| self._plugin_name = plugin or '' |
There was a problem hiding this comment.
Given plugin is always a str now, there is no need for that (unless mypy complain? but that sounds weird):
Suggested change
| self._plugin_name = plugin or '' | |
| self._plugin_name = plugin |
Note: I'm not even sure why I made plugin optional in the first place.
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This does what it says on the tin, and is intended to land in 8.1.0.
Starting as a draft, because it has existing merge conflicts already (I think they hail from #2616, but haven't checked
git blamefor all of them) and also includes part of #2633 (which needs to land in 8.0.1 instead).Checklist
make qa(runsmake lintandmake test)Notes
I probably didn't open a PR from this branch last month because I wasn't sure it was done, but now that a good 4 weeks went by and seeing this in the list of branches made me think, "Wait, what's that for?" I think I'd better get in the habit of opening draft PRs sooner rather than later. #2633 was another forgotten branch that I thought had been turned into a PR but hadn't yet.