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

Use entry points to allow packages to register plugins/extensions in various parts of Astropy #6623

Open
astrofrog opened this issue Sep 28, 2017 · 28 comments

Comments

@astrofrog
Copy link
Member

astrofrog commented Sep 28, 2017

There are currently several places where we allow or should allow packages to register extensions to Astropy. For example:

The current workflow is for a package to append something to a list or call a register function. However, this is not ideal because it means the user has to know to explicitly import the package first. For example, let's say sunpy defined a .solar file format (I'm making this up, but hey).

If I just did:

from astropy.table import Table
t = Table(...)
t.write('myfile.solar')

this wouldn't work. I'd need to do:

from astropy.table import Table
import sunpy.io  # making this up
t = Table(...)
t.write('myfile.solar')

The standard way around this kind of issue is to consider using entry points to set up a plugin system. With entry points, sunpy could then say it has plugins for file formats and provide the entry point to set up those file formats. This then allows astropy to discover plugins without the user having to import other packages explicitly (installing is enough).

To avoid a proliferation of ways of doing this, we should probably decide if we want to do this and then come up with standard entry point categories, e.g. [astropy.table.serialization], [astropy.table.formats], [astropy.coordinates.frames] and so on.

Note that if we had one entry point per Table format, we could also use entry points in the core package itself to register reader/writers instead of doing this ugly hack:

with registry.delay_doc_updates(Table):

As a bonus, for Table formats, this could mean loading entry points only when the format is requested, which could improve import-time performance.

I'm opening this here to get some initial feedback - not sure if this should become an APE, or if this + astropy-dev discussion would be enough. Maybe we can see how controversial this idea is 😆

@MSeifert04
Copy link
Contributor

MSeifert04 commented Sep 28, 2017

The main problem is documentation I guess. I don't mean documenting the entry points but updating the documentation of Table.read|write depending on the entry points. I'm not sure if that's easy to overcome but that was one of the main incentives for delay_doc_updates...

@astrofrog
Copy link
Member Author

astrofrog commented Sep 28, 2017

Well if the entry points were granular enough to be separate for reader/writer/identifier then you could generate the existing Table.read docstring without actually loading the entry points. Alternatively we could simply list the formats in the Table.read docstring without the details of reader/writer/identifier and we need to then provide a way to get format-specific detailed help anyway (which could trigger the entry point loading) - see #1011

@pllim
Copy link
Member

pllim commented Sep 28, 2017

The APE should also address conflicts where someone has installed different packages that uses the same entry point name but for different purposes (I'm also making this up but hey, could happen).

@mhvk
Copy link
Contributor

mhvk commented Sep 28, 2017

If we're thinking of custom additions, there's also:

  • custom Time formats
  • custom representations

Those two at least register just by subclassing a base format. It would seem wcs could be made with a similar kind of scheme. Coordinate frames partially does this too; the part that doesn't work like this is the frame transformation graph (perhaps there should be some frame methods that get used for defining transformations, if present). That would seem to leave mostly I/O?

@Cadair
Copy link
Member

Cadair commented Sep 28, 2017

I am all for this! Not having to document to SunPy users that they have to do a import sunpy.coordinates before using SkyCoord would be great...

@pllim pllim added the time label Sep 28, 2017
@pllim
Copy link
Member

pllim commented Sep 28, 2017

This is starting to sound like a GSoC 2018 project... 😅

@MSeifert04
Copy link
Contributor

MSeifert04 commented Sep 28, 2017

I'm a bit worried about inter-package compatibility. The entry points have to be installed but are checked at runtime. Likewise the current registering is also done at runtime.

I'm a bit worried about how one would deprecate the current way in favor of the new way. I mean the deprecation would come at runtime while the entry-points need (?) to be defined at install-time.

Probably something that needs to be addressed in the discussion/APE.

@MSeifert04
Copy link
Contributor

MSeifert04 commented Sep 28, 2017

Another question: Would that make setuptools a required runtime dependency instead of just an installation dependency or optional runtime dependency?

It's optional currently because of the modeling.fitters entry points but something like Table.read shouldn't be optional, right?

@astrofrog
Copy link
Member Author

Another question: Would that make setuptools a required runtime dependency instead of just an installation dependency or optional runtime dependency?

Yes

It's optional currently because of the modeling.fitters entry points but something like Table.read shouldn't be optional, right?

Correct - I think at this point it would be core enough to be required. Having said that it is probably the easiest required dependency to have, since if you have pip or conda then you are guaranteed to have setuptools.

I'm a bit worried about how one would deprecate the current way in favor of the new way. I mean the deprecation would come at runtime while the entry-points need (?) to be defined at install-time.

There's no need to deprecate the current way - the entry points have to call setup functions in the separate packages but those setup functions still have to do something, so e.g. an entry point for a table reader format could still call the register_reader function. The entry points would provide a convenience that avoids having users have to import the other package, but their code won't break if they continue to import e.g. sunpy. Essentially we still need to keep a traditional mechanism of registering things within astropy that can then be used by entry points.

@astrofrog
Copy link
Member Author

The more I think about this the more I realize it really should be an APE. I will try and come up with something soon. Anyone interested in helping once I have a draft? (@Cadair?)

@maartenbreddels
Copy link

See also jupyter/notebook#2894 for some discussions we're having on entry_points, the slowness is something to consider, but also see the entrypoints package mentioned there.

@Cadair
Copy link
Member

Cadair commented Oct 25, 2017

Anyone interested in helping once I have a draft? (@Cadair?)

suuuurrrreee 😉

@maartenbreddels
Copy link

Another question: Would that make setuptools a required runtime dependency instead of just an installation dependency or optional runtime dependency?

Yes

@takluyver 's entrypoints package would avoid that, I think it's a bit awkward to have setuptools as a dependency, it would be nice actually if entrypoints would be a setuptools dependency.

@takluyver
Copy link

There are two parts to setuptools - setuptools itself, which contains code for making and publishing packages, and pkg_resources, a separate module installed as part of setuptools, which is what you would use for finding entry points. My entrypoints package is an alternative to pkg_resources (for entry points, not for everything it does).

I've initiated a discussion on distutils-sig about standardising the entry points format, so that we're on solid ground for code other than setuptools to access them. After some early disagreement, I think this is now moving forwards.

Once that's done, I hope to experiment with caching entry points so finding them can be quicker. It will probably be some time before that's ready for general use, though.

@drdavella
Copy link
Contributor

On the advice of @Cadair, asdf is now using entry points to register extension classes that enable the serialization of package-specific types. This might serve as a useful example for further discussion.

@eteq
Copy link
Member

eteq commented Nov 21, 2017

In the context of #6623 (comment) above, an issue has come up that should probably be addressed in any APE on this (@Cadair?): an entry_point plugin has to be installed to be tested (we think). That breaks the usual understanding that you can run tests without installing (i.e. python setup.py test). We should include something about this in any APE about entry points.

@eteq
Copy link
Member

eteq commented Nov 21, 2017

More generally, the APE/this scheme needs to justify the added complexity/confusion of using entry points over explicit plugin approaches. While it might seem more compact to use entry points, remember both Explicit is better than implicit.and Complex is better than complicated. ...

@astrofrog
Copy link
Member Author

astrofrog commented Nov 22, 2017

@eteq - could you clarify what you mean by 'explicit plugin approaches'?

@pllim
Copy link
Member

pllim commented Oct 30, 2019

Isn't this what ASDF schema is using? Is this resolved?

@astrofrog
Copy link
Member Author

ASDF is indeed using entry points. but I think this issue touches on the broader question of whether we want to expand the use of entry points to other parts of the package, so I'd suggest keeping this open.

@mhvk
Copy link
Contributor

mhvk commented Nov 4, 2019

Yes, some form of supporting entry points still seems very worthwhile - the example of having to load sunpy before astropy.coordinates certainly suggests there has to be a better way!

@lpsinger
Copy link
Contributor

lpsinger commented May 9, 2020

Beware that entrypoints are not cached, so loading them generally requires enumerating all installed Python packages, which can be very expensive. I suggest avoiding doing anything at runtime that requires pkg_resources. Sadly, that rules out entrypoints, which otherwise would be a nice feature of setuptools. See pypa/setuptools#926.

@astrojuanlu
Copy link
Member

Some work has been done on the entry point parsing, see pypa/setuptools#510 (comment).

@mhvk
Copy link
Contributor

mhvk commented Aug 10, 2020

I've used the entry-points package for my affiliated package baseband and found it fairly straightforward and fast (it is also very small). Overall, it would still seem to be very good to have an APE with pros and cons!

@astrojuanlu
Copy link
Member

As nicely described by @saimn in #6623, with importlib.metadata (backported to Python 3.6+ as importlib-metadata) we no longer need pkg_resources or setuptools to read entry points. Also, PEP 621 standardized their syntax (among other things).

I think this extensibility is worth pursuing, in light of "consolidation" being a possible theme for Astropy 4.3.

@lpsinger
Copy link
Contributor

importlib.metadata also seems to be much faster than pkg_resources at enumerating entry points.

@pllim
Copy link
Member

pllim commented Jan 12, 2021

Re: importlib.metadata -- Might want to wait a bit more because as of Python 3.9.1, there is a big note on https://docs.python.org/3/library/importlib.metadata.html saying that, "This functionality is provisional and may deviate from the usual version semantics of the standard library." Not sure exactly what that means but I interpret it as "API not stable yet."

@saimn
Copy link
Contributor

saimn commented Jan 12, 2021

I think that means that some new features may be available only in the importlib_metadata package (not the stdlib one), as they are ported to Python's stdlib only for major release. So the stdlib version may have a more limited set of features on 3.8 fr example, but I doubt that the API change at this point for the existing features in the stdlib.
python/importlib_metadata#130 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests