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

ImportError with custom plugin #2423

Merged
merged 4 commits into from Jul 8, 2019

Conversation

3 participants
@trichter
Copy link
Member

commented Jul 2, 2019

For illustration I use obspyh5 as custom plugin, but I guess it would be the same for other plugins.

Consider the following setup:

conda create -n test obspy h5py
conda activate test
pip install obspyh5

The following script is working:

import obspy
import obspyh5

The following script is not working in obspy master.

import obspyh5
Traceback (most recent call last):
  File "/home/eule/anaconda/envs/workhorse/lib/python3.6/site-packages/pkg_resources/__init__.py", line 2442, in resolve
    return functools.reduce(getattr, self.attrs, module)
AttributeError: module 'obspyh5' has no attribute 'readh5'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test.py", line 1, in <module>
    import obspyh5
  File "/home/eule/dev/obspyh5/obspyh5.py", line 26, in <module>
    from obspy.core import Trace, Stream, UTCDateTime as UTC
  File "/home/eule/dev/obspy/obspy/__init__.py", line 59, in <module>
    read.__doc__ % make_format_plugin_table("waveform", "read", numspaces=4)
  File "/home/eule/dev/obspy/obspy/core/util/base.py", line 522, in make_format_plugin_table
    func = buffered_load_entry_point(*ep_list)
  File "/home/eule/dev/obspy/obspy/core/util/misc.py", line 644, in buffered_load_entry_point
    _ENTRY_POINT_CACHE[hash_str] = load_entry_point(dist, group, name)
  File "/home/eule/anaconda/envs/workhorse/lib/python3.6/site-packages/pkg_resources/__init__.py", line 489, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/home/eule/anaconda/envs/workhorse/lib/python3.6/site-packages/pkg_resources/__init__.py", line 2843, in load_entry_point
    return ep.load()
  File "/home/eule/anaconda/envs/workhorse/lib/python3.6/site-packages/pkg_resources/__init__.py", line 2434, in load
    return self.resolve()
  File "/home/eule/anaconda/envs/workhorse/lib/python3.6/site-packages/pkg_resources/__init__.py", line 2444, in resolve
    raise ImportError(str(exc))
ImportError: module 'obspyh5' has no attribute 'readh5'

@medlin01GA Thanks for spotting this mysterious bug.

@trichter

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2019

It seems calling load_entry_point on a module during import of the module itself causes this problem.
I solved the problem with this diff

--- a/obspy/core/util/base.py
+++ b/obspy/core/util/base.py
@@ -29,7 +29,7 @@ import numpy as np
 import pkg_resources
 import requests
 from future.utils import native_str
-from pkg_resources import iter_entry_points
+from pkg_resources import get_entry_info, iter_entry_points
 
 from obspy.core.util.misc import to_int_or_zero, buffered_load_entry_point
 
@@ -519,8 +519,8 @@ def make_format_plugin_table(group="waveform", method="read", numspaces=4,
     for name, ep in eps.items():
         module_short = ":mod:`%s`" % ".".join(ep.module_name.split(".")[:3])
         ep_list = [ep.dist.key, "obspy.plugin.%s.%s" % (group, name), method]
-        func = buffered_load_entry_point(*ep_list)
-        func_str = ':func:`%s`' % ".".join((ep.module_name, func.__name__))
+        entry_info = str(get_entry_info(*ep_list))
+        func_str = ':func:`%s`' % entry_info.split(' = ')[1].replace(':','.')
         mod_list.append((name, module_short, func_str))
 
     mod_list = sorted(mod_list)

I guess this should be OK?

@trichter

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2019

+DOCS +DOC

@trichter trichter added the bug label Jul 2, 2019

@trichter trichter added this to the 1.2.0 milestone Jul 2, 2019

@trichter trichter added this to Waiting on CI in Release 1.2.0 Jul 2, 2019

@trichter

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2019

I checked the 6 different plugin tables (Event, Inventory, Stream, read and write) in the documentation. All look good with working links.

@trichter trichter moved this from Waiting on CI to Waiting for final manual validation by Core Dev in Release 1.2.0 Jul 3, 2019

@d-chambers

This comment has been minimized.

Copy link
Member

commented Jul 3, 2019

Hey @trichter ,
I just took a quick look at this. It seems like a good fix, but unfortunately our CI is a complete mess again...

Anything else you want to do with it?

@ThomasLecocq

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2019

aha, I noticed the same with obspy-dmx I wrote recently, good you found it !

@trichter

This comment has been minimized.

Copy link
Member Author

commented Jul 4, 2019

@d-chambers No, nothing to do is left. I think it is ready to merge. And apart from fixing the ImportError it only affects the documentation, which I checked.

@ThomasLecocq One of the bugs which get triggered only under some rare circumstances.

@trichter

This comment has been minimized.

Copy link
Member Author

commented Jul 4, 2019

I think this PR also reduces the import time of obspy somewhat.

@d-chambers

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

Ok, I have looked over the CI runs as well as run a few tests locally and I feel ok about merging this.

However, I have uncovered one issue that is partially related. If someone tries to re-import obspy it fails because the __doc__ strings have already been formatted when obspy.__init__ is run again. For example, on python 3.7:

import importlib

import obspy

importlib.reload(obspy)

raises a TypeError.

I tried fixing it (just pushed here), do you mind taking a look at it. After that, it should be good to merge.

@trichter

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2019

Thanks for running some tests and having a look.
Your changes look good.
I have some suggestions:

  • be verbose and call the function _add_format_plugin_table
  • move it to obspy.core.util.base
  • Would something like this work, too?
def _add_format_plugin_table(func, group, method, numspaces=4):
    if '%s' in func.__doc__:
        func.__doc__ = func.__doc__ % make_format_plugin_table(
            group, method, numspaces=numspaces)
@d-chambers

This comment has been minimized.

Copy link
Member

commented Jul 6, 2019

@trichter sure that works too. I should have time to make the changes early next week, or feel free to change how you see fit, then we will merge it.

@trichter

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

@d-chambers Thanks, it is ready for merging now.

@d-chambers d-chambers merged commit 587d081 into master Jul 8, 2019

1 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
docs-buildbot Build succeeded, but there are warnings/errors:
Details
docker-testbot docker testbot results not available yet
ci/circleci Your tests passed on CircleCI!
Details
@d-chambers

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

Ok, I ran one last set of tests on py27 and py37 and the failures are unrelated, merging.

@d-chambers d-chambers deleted the custom_plugin_import branch Jul 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.