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

DeprecationWarning: SelectableGroups dict interface is deprecated. Use select. #298

Closed
CaselIT opened this issue Mar 29, 2021 · 19 comments
Closed

Comments

@CaselIT
Copy link

CaselIT commented Mar 29, 2021

In sqlalchemy we are using this library for python_version<"3.8", using only entry_points.

Since version 3.7.1 this library is no longer a drop in replacement to the stdlib version, since it will raise deprecation warnings when accessing the dict interface. The stdlib returns a plain dict so it's a bit hard to use a non-dict interface.

I don't think sqlalchemy is the only package that uses this library as a drop in replacement if older python version, so it would be useful it the dict interface were not deprecated.

@jaraco
Copy link
Member

jaraco commented Mar 29, 2021

The intention is to provide a transition away from the dict interface, because the dict interface removes important information (ordering) and is less performant for common cases (iterate over entry points for a group). See #284 and #282 for more detail.

Libraries have a few options:

  • Pin to importlib_metadata < 3.9.
  • Suppress the deprecation warnings and pin to importlib_metadata < 4.
  • Update to importlib_metadata >= 3.6.

I recommend the last option, as it is the most future-proof and provides the nicest interface. Python 3.10 includes this behavior so you can rely on stdlib again in that version if you wish. Yes, it requires an additional dependency on two Python versions (3.8, 3.9). What's the harm in that?

@jaraco
Copy link
Member

jaraco commented Mar 29, 2021

Note that importlib_metadata 3.6 is only supported on Python 3.6 or later, so projects still supporting Python 2 are probably better off to take one of the first two options.

@jaraco
Copy link
Member

jaraco commented Mar 29, 2021

I've considered backporting the SelectableGroups functionality to importlib_metadata 2.x to give backward compatibility for that feature back to Python 2.7. If the demand is there, let me know.

@zzzeek
Copy link

zzzeek commented Mar 30, 2021

we want to use whatever is not deprecated. I'm not really deep in this issue but we are using the entry_points method right in Python 3, which is documented as "returns a dict" and I dont see anything about it being deprecated? is importlib_metadata trying to be compatible with what's documented at https://docs.python.org/3/library/importlib.metadata.html#entry-points ? AFAICT we are calling a single method, "get()". So.... we don't call get() anymore? what do we call?

@zzzeek
Copy link

zzzeek commented Mar 30, 2021

also, we're a library, so pinning is not an option, and messing with the warnings filter is kind of bad also, so those aren't options.

@zzzeek
Copy link

zzzeek commented Mar 30, 2021

OK the method instead of .get(X) is .select(group=X). This interface is not even in python 3.9 !!

File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/util/langhelpers.py", line 334, in load
    for impl in compat.importlib_metadata.entry_points().select(
AttributeError: 'dict' object has no attribute 'select'

I really think importlib_metadata shouldn't be emitting warnings like this and should just keep everything the same as it is in the Python versions it purports to provide compatibility towards. We can just work around this as always with using different methods for different python versions but now we literally have to have rules not only for Python versions but for which importlib_metadata is installed for an older version.

@jaraco
Copy link
Member

jaraco commented Mar 30, 2021

also, we're a library, so pinning is not an option, and messing with the warnings filter is kind of bad also, so those aren't options.

That's a dimension I haven't fully considered yet. That's an important consideration I need to ponder more. This factor may demand an extended deprecation period.

entry_points method right in Python 3, which is documented as "returns a dict" and I dont see anything about it being deprecated?

Right. The new interface was introduced only recently. And the compatibility changes have landed in Python 3.10. I'd like for the deprecation also to land in Python 3.10, but I want to get feedback in importlib_metadata first to validate my assumptions and make sure there's a viable transition plan.

Please bear with this change. I'll work with sqlalchemy to devise a proper recommendation and solution, even if that means pulling the deprecation warning. Is there urgency to the warning, or can the warning remain for now while I consider the concerns?

@zzzeek
Copy link

zzzeek commented Mar 30, 2021

our test suite is failing right now due to this change since we raise on warnings, but the whole thing is in a "compat" module that has checks so I can fix this on my end by just testing for ".select()" being present etc. and I'll probably do that since at least one release here will cause warnings, we definitely get bug reports for any warnings emitted by anything.

@CaselIT
Copy link
Author

CaselIT commented Mar 30, 2021

An alternative, since the returned object is still a dict it to use dict.get(retval, group). This should work without emitting the warning.

Is the long term plan to still return a dict?
Otherwise it's probably better to wrap entry_points in a compat function

@jaraco
Copy link
Member

jaraco commented Mar 30, 2021

I have a plan to ease use-cases like sqlalchemy's. I'll create a backports.entry_points_selectable project that does the following:

  • Requires importlib_metadata prior to 3.8.
  • Supplies SelectableGroups (the compatibility shim) for environments that don't have importlib_metadata older than 3.6 or aren't on older Pythons that don't support importlib_metadata 3.6.
  • Presents a simple entry_points callable that applies the compatibility shim when needed.

This approach is very similar to the one zzzeek proposed, but made available for any library to use. It will allow the caller to simply require the backport and get an entry_points interface that's compatible with the preferred interface.

For more information, see the changelog for 3.6 and the compatibility note in the entry points docs.

I'd recommend to simply suppress the warning for now and I should have the backport out later this week (or this weekend at the latest).

Is the long term plan to still return a dict?

No. The point of this transition is to eliminate the dict and provide interfaces that more simply match with what most use-cases demand. If someone demonstrates a need for a dict, they can construct one using the technique that was previously the default and if it's a common-enough case, I'd be happy to supply that as an adapter on the entry_points result, which now aims to return a lower-level abstraction (simple iterable rather than grouped items).

An alternative, since the returned object is still a dict it to use dict.get(retval, group). This should work without emitting the warning.

I'd recommend against that and instead just suppress the deprecation warning, as suppressing the warning is clearer than bypassing it.

@CaselIT
Copy link
Author

CaselIT commented Mar 30, 2021

Thanks for the effort!

For the moment Mike has proposed this patch here: https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2690/1/lib/sqlalchemy/util/compat.py

Since our usage is really basic that should suffice for the moment

@jaraco
Copy link
Member

jaraco commented Mar 30, 2021

I've created https://github.com/jaraco/backports.entry_points_selectable and it should be publishing 1.0.0 shortly. Please consider trying it out. Thanks again for your patience.

@zzzeek
Copy link

zzzeek commented Mar 30, 2021

We are going the safest route which is to detect when .select() is available on this object and calling that. as mentioned before, we can't suppress warnings and we dont want to touch deprecated APIs ever. that said I think it's inappropriate for a temporary compatiblitiy shim like importlib_metadata, which IIUC nobody will be using once Python 3.8 is their bottom version, to be seeking backwards-incompatible optimizations and deprecation paths at all.

@jaraco
Copy link
Member

jaraco commented Mar 30, 2021

Right. That's what backports.entry_points_selectable provides - forward compatibility even for importlib.metadata on python 3.8-3.9 and compatibility for importlib_metadata on Python 2.7+, but preferring the native implementation when present. It's main goal is to ensure that .select is present and handle the compatibility dance but ultimately rely on preferred behavior when available.

My hope is this project will save sqlalchemy and many other projects the burden of thinking of about compatibility and to bear that burden for them.

@jaraco
Copy link
Member

jaraco commented Mar 30, 2021

Also, I don't mean to sound insistent. You're of course welcome to manage the issue in sqlalchemy however you like. I'm going to resolve the issue as I presume backports.entry_points_selectable will address the issue generally. Please let me know if there are other concerns where I may be able to help.

@jaraco jaraco closed this as completed Mar 30, 2021
@jaraco jaraco pinned this issue Mar 30, 2021
@CaselIT
Copy link
Author

CaselIT commented Mar 30, 2021

thanks @jaraco we will look into the new package for sqlalchemy.

the documentation in the readme may need an update also to make the compatibility package more discoverable.

@jaraco
Copy link
Member

jaraco commented Mar 30, 2021

See PyCQA/flake8!472 for an illustration of the backport in use.

@asottile
Copy link
Contributor

that was closed, this is a bad approach to force backport packages to make an api break.

please keep the dict api

@asottile
Copy link
Contributor

this feedback was already produced in #278 I guess -- sad to see you went with a break

@jaraco jaraco unpinned this issue Aug 26, 2021
flokli added a commit to flokli/nixpkgs that referenced this issue Oct 15, 2021
Tests fail on Python 3.7 due to importlib using a deprecated interface.

Context: python/importlib_metadata#298
flokli added a commit to flokli/nixpkgs that referenced this issue Oct 15, 2021
Tests fail on Python 3.7 due to importlib using a deprecated interface.

Context: python/importlib_metadata#298
martinhoyer added a commit to martinhoyer/tmt that referenced this issue May 22, 2023
Adds forward-compatibility for entry_points.
Fixes deprecation warnings.
See: python/importlib_metadata#298
martinhoyer added a commit to martinhoyer/tmt that referenced this issue May 22, 2023
The “selectable” entry points were introduced in importlib_metadata 3.6 and Python 3.10.
While this could be solved by adding entry_points>=3.6 for python<3.10 as a dependency, python3-importib-metadata el8 package currently only have version 0.23.
Fixes deprecation warnings.
See: python/importlib_metadata#298
happz pushed a commit to martinhoyer/tmt that referenced this issue Jun 1, 2023
The “selectable” entry points were introduced in importlib_metadata 3.6 and Python 3.10.
While this could be solved by adding entry_points>=3.6 for python<3.10 as a dependency, python3-importib-metadata el8 package currently only have version 0.23.
Fixes deprecation warnings.
See: python/importlib_metadata#298
happz pushed a commit to teemtee/tmt that referenced this issue Jun 1, 2023
The “selectable” entry points were introduced in importlib_metadata 3.6 and Python 3.10.
While this could be solved by adding entry_points>=3.6 for python<3.10 as a dependency, python3-importib-metadata el8 package currently only have version 0.23.
Fixes deprecation warnings.
See: python/importlib_metadata#298
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

No branches or pull requests

4 participants