-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
build, meta: drop Python 3.7 support, checks, and CI job #2500
Conversation
Linking to just `/3/` is admittedly risky, because item/anchor locations can change across Python versions. Bumping the links from 3.7 to 3.11 gives us a few more years of not needing to touch them.
Can't drop the `importlib_metadata` requirement yet, though. We use some stuff elsewhere that isn't stable in stdlib until py3.10.
Played with using intentionally wrong types in the definition of a `PluginMetaDescription` and running `mypy` against it, but didn't see any (new) errors flagged over that. Maybe I did it wrong, but it took enough work shuffling docstrings around that I'm committing this anyway.
The test covering `_chunks()` still passes, so hooray!
Regular `dict` is promised to maintain insertion order as of Python 3.7 (which means this could have been done sooner, but it got forgotten, so we're doing it at the point of dropping Python <3.8 instead).
Well, now the latest version of Line 1782 in e63afba
The name Line 1766 in e63afba
Upstream seems to consider this as "Working as intended" (PyCQA/pyflakes#780), so I guess we'll have to |
55fe4a7
to
f675d27
Compare
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.
LGTM, most of this is pretty straightforward housekeeping.
Not doing exactly as the comment says because different Rule subclasses pass aliases as either tuple or list depending on how whoever wrote the subclass was feeling that day. Instead, let's simplify the code to do everything in one step, and unpack the `aliases` iterable so its type won't even matter. Co-authored-by: SnoopJ <snoopjedi@gmail.com>
Having whittled down most of the open threads from @SnoopJ and myself, I figured it's worth seeing if Exi has free time later this week(end) to chime in. |
`importlib.metadata` officially starts existing in Python 3.8, and we are removing support for Python 3.7 before Sopel 8's stable release. That means we can use the stdlib approach here to get the version string for an entry-point plugin, and drop another `import importlib_metadata`. I can't wait for Python 3.9's EOL... We can remove `importlib_metadata` completely then.
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.
LGTM, a couple other grep results:
modules/tld.py
has a "py<3.5" comment- I think
importlib_metadata
can be removed from a few more places, unless 3.8 doesn't haveimportlib.metadata.EntryPoint()
It does have |
My pattern missed that because of the |
As of Python 3.5, we can do this. It's overdue.
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.
It does have
EntryPoint
but I believe the behavior ofentry_points()
changes subtly between 3.9 and 3.10 which keeps the dependency around? I don't know the details myself, but see this comment
I read that as being about entry_points(filter=)
only, not the EntryPoint
class itself, but I don't have py3.8 handy to test with.
Either way, we still need the dependency for now, so eh. 🚀
importlib.metadata.EntryPoint *shouldn't* differ from Python 3.8 onward, unless there's something missing from the documentation. If this breaks stuff on older Python releases (I'm on 3.10 locally), we will defer this change until we drop 3.9 as planned per the code comments.
I decided to throw replacing |
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.
I love importlib.metadata
. I love the walrus operator (:=
). I love removing Py3.7.
Description
This fulfills the last "Finally letting the dead snakes lie." part of the 8.0.0 milestone's description. Python 3.7 was officially EOL'd as of a month ago, on June 27.
Checklist
make qa
(runsmake quality
andmake test
)make test
etc. (Now it needs a rebase for retesting.)Conflict notes
@SnoopJ and @Exirel arrived at similar conclusions for dealing with aliases in
plugins/rules.py
; this has a conflict with 098d8c3, but the approaches are quite similar. That and b0693b7 are both from #2471, which touched a bunch of stuff (so I knew the risk of conflicts with this relatively long-lived housekeeping branch was inevitable).The only other little conflict is more directly my fault. While I set @SnoopJ loose on my todo list for #2471 to get it done faster, #2480—specifically, 85f8b82—is all me. 🙈
Guess I have to rebase this one, huh? Damn, I love merging branches that diverged long before
master
because the history graph looks cool, but sometimes it's just not meant to be. 😅Special thanks
…to the other maintainers for humoring me when I said I wanted to reach #2500 and make this PR a numerical milestone as well as a development one. You're all awesome for tossing in extra issues & PRs to get us here! ❤️