Repository navigation
Change entrypoint to be a toml file (or similar) #24
Description
Activity
Hmmm, this seems trickier than I expected. First I thought there may be a utility to just do this, then I thought this might work:
def get_entrypoint(ep): mod, _, filename = ep.value.partition spec = importlib.util.find_spec(mod) reader = spec.loader.get_resource_reader(spec.name) with reader.open_resource(filename) as f: return json.load(f) # or toml rather.
But that still seems to import the module in which the entry-point is contained (i.e.
ep.value = "mymodule:filename.json"there).I suppose there is likely some way to dig even deeper and avoid the import.
This way, it seems the entry-point still needs to be its own mini-package. Unless we shoehorn most info into the entry-point value itself, but that doesn't seem desireable?
I half wonder if there might be a way to probe for any import, but I don't really want to use audithooks or so :).
Maybe I am missing an obvious approach here?
EDIT: ah, maybe there is a better way, but the pattern above works fine. The
modjust must be top level, otherwise the higher ones do get imported (i.e. no.must be contained).cc @Czaki
Yeah, so see gh-28, which implements this (need to fix CI/dependencies but locally it's fine).
I do like the new update of functions (this update was always optional) with
tomlkitvia:python -mspatch update-entrypoints ....
And I generally like the syntax (the biggest question is, if the loading of the entry-point itself is a bit slower, but it seems very unlikely).However, I had to disable the
.tomlvalidator because this suggestion of using a toml is clearly not how anyone else uses entry-points or how they are intended to be used.So while in generally, I like the fact that with some dance I can avoid any imports (and thus don't need a second package just for that). And I think that the way I read the file from the package will work cross platform and also if the package is for example zipped...
But, the fact remains that the entry-point value is as such meant to hold an arbitrary piece of information, so is there a clearer way to do this?
Maybe, the solution is just to ignore the problem and see if I can get the pyproject toml validator to add a way to
# noqathe "bad" entry-point?!The alternative (that I might just do if no one has an opinion), would be force the
.tomlfile to be top-level inside the package and follow a naming scheme like<entry_point_group>_<name>.toml.
I don't really like that (on grounds of this just being more confusing), but...Sample script to iterate over napari plugins static description without plugin import:
from importlib import metadata, util from pathlib import Path from tomllib import loads as toml_loads from yaml import safe_load as yaml_loads def get_file_from_entry_point(ep: metadata.EntryPoint) -> dict: try: module, file = ep.value.split(':') except TypeError: return {} spec = util.find_spec(module) if not spec: return {} if spec.submodule_search_locations: file_path = Path(spec.submodule_search_locations[0]).resolve() / file elif spec.origin: file_path = Path(spec.origin).resolve() / file else: return {} if not file_path.exists(): return {} if file_path.suffix == '.yaml': with file_path.open() as f: return {'module': module, 'file': file, 'data': yaml_loads(f)} elif file_path.suffix == '.toml': with file_path.open() as f: return {'module': module, 'file': file, 'data': toml_loads(f)} return {} ep_list = metadata.entry_points(group="napari.manifest") for ep in ep_list: print(get_file_from_entry_point(ep))
You may see napari plugins definition here: https://napari.org/stable/plugins/building_a_plugin/first_plugin.html#add-a-napari-yaml-manifest
So the answer is that you are (ab)using the
valuefield as a filename. My observation is:pyproject-validatorwill complain about it (if the file includes a/)- New
importlib_metadata(and thus possibly the next/very new Python) may also complain.
I had tried asking on python-discuss. But if this is prior art, I think we should create an issue for
importlib_metadata! Otherwise this may well break napari in the future.It is actually even worse, because the error is already raised by
importlib_metadata.entrypoints(group=...)which seems wrong either way though: I expectentrypoint.load()to fail, but the discovery must not fail.But... I think it means that my PR is good to go, we have to take this up with the powers to be (I don't care much about the validator, but
importlib_metdatamust gte an issue).pyproject-validatorwill complain about it (if the file includes a/)Why the file even may contain
/in the name? I do not see such a scenario.After the
:we expect a filename, not a file path. The file should be in the module specified before:.@Czaki the reason why I am using
/in the path, is that my observation is that even:spec = util.find_spec(module)ends up doing a full
import moduleof the top level module, which defies the whole point of avoiding loading a Python path?(I am also slightly worried that your approach rather than mine of using:
reader = spec.loader.get_resource_reader(spec.name) with reader.open_resource(filename) as f:
may not work with zipped packages, which I think exist. But I since it seems already the
util.find_specstep causes the top-level to be imported, that doesn't actually matter.)ends up doing a full
import moduleof the top level module, which defies the whole point of avoiding loading a Python path?Oh. I see. I need to check our code as I paste some extraction here.
I do not see a plugin that hasnapari.yamlnot on the top level.
And based on documentation:If name is for a submodule (contains a dot), the parent module is automatically imported.
If the file is on the top level, then it is not imported.
So modification:
def get_file_from_entry_point(ep: metadata.EntryPoint) -> dict: try: module, file = ep.value.split(':') except TypeError: return {} base_module, *sub_modules = module.split('.') spec = util.find_spec(base_module) if not spec: return {} if spec.submodule_search_locations: file_path = Path(spec.submodule_search_locations[0]).resolve().joinpath(sub_modules) / file elif spec.origin: file_path = Path(spec.origin).resolve().joinpath(sub_modules) / file else: return {} if not file_path.exists(): return {} if file_path.suffix == '.yaml': with file_path.open() as f: return {'module': module, 'file': file, 'data': yaml_loads(f)} elif file_path.suffix == '.toml': with file_path.open() as f: return {'module': module, 'file': file, 'data': toml_loads(f)} return {}
It requires the modules to be real module directories. But it is not a problematic assumption for me.
I do not have experience with zip packages. Need to read.
Reacted by Sebastian BergI do not have experience with zip packages. Need to read.
Yeah, me neither, just think it's a thing and I stumbled on
with reader.open_resource(filename) as f:which looked like should be safe against it (and also saves thefile_pathresolving).I agree, one can use
.instead of/and it's all not particularly problematic. Just feels a bit thatimportlibshouldn't enforce this dance, so opened an issue inimportlib_metadatafor now, let's see.
(Mostly, I would like to hear if someone is horrified or they think: oh, yeah we should just bless this use. Chance is nobody will care in practice, which is also OK.)
The entry-point shouldn't be actual python code, that way we can really be 100% sure of no expensive imports (with the exception of abstract types).
Somehow, I wasn't aware that you can do that :).