glue-viz / glue

Linked Data Visualizations Across Multiple Files
http://glueviz.org
Other
742 stars 153 forks source link

DEP: drop runtime dependency on `setuptools` #2511

Open neutrinoceros opened 2 months ago

neutrinoceros commented 2 months ago

Description

This is a follow up to #2365 At the moment this breaks tests because mpl-scatter-density has an undeclared runtime dependency on setuptools (addressed at https://github.com/astrofrog/mpl-scatter-density/pull/43)

I also note that the following comment https://github.com/glue-viz/glue/blob/e9df453717b05a632737fbf7a1cefd760459af3c/glue/main.py#L75-L87

which was introduced in #1487, has been irrelevant since #2365 (at least in part) since it mentions behavior details that were specific to setuptools. Should this comment be removed entierely or just rephrased ?

neutrinoceros commented 2 months ago

are you sure that these kind of errors should not happen anymore?

I don't know if there's a test to exercise this condition, but I tried replacing module = import_module(item.module) with module = item.load() and found it broke some tests for a different reason: EntryPoint.load doesn't always return a module, which is actually documented in this function's docstring:

        """Load the entry point from its definition. If only a module
        is indicated by the value, return that module. Otherwise,
        return the named object.
        """

so it stills seems simpler/safer to use import_module here, but I think we should update the comment to reflect the real reason. Should I do it here ?