ejeschke / ginga

The Ginga astronomical FITS file viewer
BSD 3-Clause "New" or "Revised" License
121 stars 77 forks source link

Replace pkg_resources #964

Closed ejeschke closed 3 years ago

ejeschke commented 3 years ago
ejeschke commented 3 years ago

We have one externally installed plugin that we distribute here at Subaru. I just tested it with the fix and it works perfectly.

ejeschke commented 3 years ago

The uses of pkg_resources in the __init__.py 's I don't understand. They all look like this:

try:
    # As long as we're using setuptools/distribute, we need to do this the
    # setuptools way or else pkg_resources will throw up unncessary and
    # annoying warnings (even though the namespace mechanism will still
    # otherwise work without it).
    # Get rid of this as soon as setuptools/distribute is dead.
    __import__('pkg_resources').declare_namespace(__name__)
except ImportError:
    pass
__path__ = __import__('pkgutil').extend_path(__path__, __name__)
ejeschke commented 3 years ago

@pllim, I don't understand why we need to declare the namespace or even this line:

__path__ = __import__('pkgutil').extend_path(__path__, __name__)

We only seem to have this in about 4 __init__.py files. Can we just remove it all? Any idea what this might have been added for?

ejeschke commented 3 years ago

This gives some hints, but I'm still not clear on why it is necessary.

ejeschke commented 3 years ago

This suggests that if this is being used for some kind of namespace packaging/importing, from Python 3.3 onward we can just delete the __init__.py file to get the same behavior.

ejeschke commented 3 years ago

@pllim, I believe I have incorporated the suggested changes. Have a look/test when you have a chance.

ejeschke commented 3 years ago

Looking back at #169, I'm intrigued by the (now) Python 3 feature of "auto" namespaces via removing the __init__.py files from certain directories. For example, if we remove it from ginga.rv.plugins, can we do away with the entry points business required for separately distributed plugins? Would it be sufficient for a separately distributed plugin to simply declare that hierarchy? Is it straightforward to then query that namespace from ginga?

@embray, can you lend your Python expertise to shed any light on that?

pllim commented 3 years ago

Thanks, @ejeschke . This is on my radar but I just need to find some time to test this patch downstream. Thanks for your patience!

ejeschke commented 3 years ago

No hurry, @pllim. I guess that if all your plugins load then that is the main test.

ejeschke commented 3 years ago

Does this patch still work for you?

Yes, it does, thanks! I've pushed up the change.

ejeschke commented 3 years ago

Thanks, @pllim!

embray commented 3 years ago

Sorry I just saw this comment: https://github.com/ejeschke/ginga/pull/964#issuecomment-859931957

Personally, I don't have a deep insight into it, since I haven't encountered this use case much in the wild. IIRC what happens if you install multiple packages that use the same namespace package, they will all be merged into one top-level package like.

I did an experiment on this: I created two projects foo.a and foo.b with the following structure:

- setup.cfg
- pyproject.toml
- foo
  - a
    - __init__.py

with setup.cfg containing:

[metadata]
name = foo.a
version = 0.1

[options]
packages = foo.a

(incidentally, if I just put packages = find:, setuptools does not recognized foo as being a potential namespace package, so I had to explicitly write packages = foo.a).

The same structure is repeated but for foo.b.

When I install both packages, you do get the combined namespace package in site-packages:

- foo
  - a
    - __init__.py
  - b
    - __init__.py

But I don't know that there's an "obvious" way to query this.

If you do

>>> import foo
>>> foo.__path__
_NamespacePath(['/home/embray/.virtualenvs/tmp-3d35b7a92973a33e/lib/python3.7/site-packages/foo'])

I don't know what the significance is of the special _NamespacePath class, but it must be used internally by the import system to recognize that this is a package without an __init__.py.

Beyond that, the only way to query what possible subpackages are available would be to loop over directory entries in foo.__path__.

I still think entry_points are the best way to clearly register pluggable extensions to a package.

ejeschke commented 3 years ago

Thanks for the thorough answer, @embray! Sounds like it could work for such purposes, although (as you say) there is no obviously correct way to query the the namespace. I wonder if something like that might develop however; the entry points business seems tied to setuptools, whereas this now feels like the "official way" to make a namespace package that is not tied to packaging...