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

Running tests from astropy.test() fails when asdf-astropy is installed #16165

Closed
WilliamJamieson opened this issue Mar 6, 2024 · 10 comments · Fixed by #16179
Closed

Running tests from astropy.test() fails when asdf-astropy is installed #16165

WilliamJamieson opened this issue Mar 6, 2024 · 10 comments · Fixed by #16179

Comments

@WilliamJamieson
Copy link
Contributor

On Python 3.12.2 if I install astropy in a clean environment using an editable install with the test and all dependencies included and then run

import astropy
astropy.test()

The tests fail to run with the error during test collection:

File ~/Workspaces/astropy/astropy/tests/runner.py:271, in TestRunnerBase.make_test_runner_in.<locals>.test(**kwargs)
    269 @wraps(runner.run_tests, ("__doc__",))
    270 def test(**kwargs):
--> 271     return runner.run_tests(**kwargs)

File ~/Workspaces/astropy/astropy/tests/runner.py:598, in TestRunner.run_tests(self, **kwargs)
    592 def run_tests(self, **kwargs):
    593     # This prevents cyclical import problems that make it
    594     # impossible to test packages that define Table types on their
    595     # own.
    596     from astropy.table import Table  # noqa: F401
--> 598     return super().run_tests(**kwargs)

File ~/Workspaces/astropy/astropy/tests/runner.py:254, in TestRunnerBase.run_tests(self, **kwargs)
    252 with set_temp_config(astropy_config, delete=True):
    253     with set_temp_cache(astropy_cache, delete=True):
--> 254         return pytest.main(args=args, plugins=plugins)

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/config/__init__.py:156, in main(args, plugins)
    154 try:
    155     try:
--> 156         config = _prepareconfig(args, plugins)
    157     except ConftestImportFailure as e:
    158         exc_info = ExceptionInfo.from_exc_info(e.excinfo)

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/config/__init__.py:338, in _prepareconfig(args, plugins)
    336             else:
    337                 pluginmanager.register(plugin)
--> 338     config = pluginmanager.hook.pytest_cmdline_parse(
    339         pluginmanager=pluginmanager, args=args
    340     )
    341     return config
    342 except BaseException:

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/pluggy/_hooks.py:501, in HookCaller.__call__(self, **kwargs)
    499 firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
    500 # Copy because plugins may register other plugins during iteration (#438).
--> 501 return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/pluggy/_manager.py:119, in PluginManager._hookexec(self, hook_name, methods, kwargs, firstresult)
    110 def _hookexec(
    111     self,
    112     hook_name: str,
   (...)
    117     # called from all hookcaller instances.
    118     # enable_tracing will set its own wrapping function at self._inner_hookexec
--> 119     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)

    [... skipping hidden 2 frame]

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/helpconfig.py:105, in pytest_cmdline_parse()
    103 @pytest.hookimpl(wrapper=True)
    104 def pytest_cmdline_parse() -> Generator[None, Config, Config]:
--> 105     config = yield
    107     if config.option.debug:
    108         # --debug | --debug <file.log> was provided.
    109         path = config.option.debug

    [... skipping hidden 1 frame]

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/config/__init__.py:1096, in Config.pytest_cmdline_parse(self, pluginmanager, args)
   1092 def pytest_cmdline_parse(
   1093     self, pluginmanager: PytestPluginManager, args: List[str]
   1094 ) -> "Config":
   1095     try:
-> 1096         self.parse(args)
   1097     except UsageError:
   1098         # Handle --version and --help here in a minimal fashion.
   1099         # This gets done via helpconfig normally, but its
   1100         # pytest_cmdline_main is not called in case of errors.
   1101         if getattr(self.option, "version", False) or "--version" in args:

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/config/__init__.py:1449, in Config.parse(self, args, addopts)
   1443 assert (
   1444     self.args == []
   1445 ), "can only parse cmdline args at most once per Config object"
   1446 self.hook.pytest_addhooks.call_historic(
   1447     kwargs=dict(pluginmanager=self.pluginmanager)
   1448 )
-> 1449 self._preparse(args, addopts=addopts)
   1450 # XXX deprecated hook:
   1451 self.hook.pytest_cmdline_preparse(config=self, args=args)

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/config/__init__.py:1321, in Config._preparse(self, args, addopts)
   1317 self.known_args_namespace = self._parser.parse_known_args(
   1318     args, namespace=copy.copy(self.option)
   1319 )
   1320 self._checkversion()
-> 1321 self._consider_importhook(args)
   1322 self.pluginmanager.consider_preparse(args, exclude_only=False)
   1323 if not os.environ.get("PYTEST_DISABLE_PLUGIN_AUTOLOAD"):
   1324     # Don't autoload from setuptools entry point. Only explicitly specified
   1325     # plugins are going to be loaded.

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/config/__init__.py:1225, in Config._consider_importhook(self, args)
   1223         mode = "plain"
   1224     else:
-> 1225         self._mark_plugins_for_rewrite(hook)
   1226 self._warn_about_missing_assertion(mode)

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/config/__init__.py:1246, in Config._mark_plugins_for_rewrite(self, hook)
   1238 package_files = (
   1239     str(file)
   1240     for dist in importlib.metadata.distributions()
   1241     if any(ep.group == "pytest11" for ep in dist.entry_points)
   1242     for file in dist.files or []
   1243 )
   1245 for name in _iter_rewritable_modules(package_files):
-> 1246     hook.mark_rewrite(name)

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:267, in AssertionRewritingHook.mark_rewrite(self, *names)
    263     mod = sys.modules[name]
    264     if not AssertionRewriter.is_rewrite_disabled(
    265         mod.__doc__ or ""
    266     ) and not isinstance(mod.__loader__, type(self)):
--> 267         self._warn_already_imported(name)
    268 self._must_rewrite.update(names)
    269 self._marked_for_rewrite_cache.clear()

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:274, in AssertionRewritingHook._warn_already_imported(self, name)
    271 def _warn_already_imported(self, name: str) -> None:
    272     from _pytest.warning_types import PytestAssertRewriteWarning
--> 274     self.config.issue_config_time_warning(
    275         PytestAssertRewriteWarning(
    276             "Module already imported so cannot be rewritten: %s" % name
    277         ),
    278         stacklevel=5,
    279     )

File ~/.pyenv/versions/3.12.2/envs/astropy/lib/python3.12/site-packages/_pytest/config/__init__.py:1489, in Config.issue_config_time_warning(self, warning, stacklevel)
   1487     warnings.simplefilter("always", type(warning))
   1488     apply_warning_filters(config_filters, cmdline_filters)
-> 1489     warnings.warn(warning, stacklevel=stacklevel)
   1491 if records:
   1492     frame = sys._getframe(stacklevel - 1)

PytestAssertRewriteWarning: Module already imported so cannot be rewritten: asdf

However, when I invoke pytest from the CLI using either pytest (in the astropy repo root directory) or pytest --pyargs astropy, the tests run and pass without issue.

I suspect this has to do with some interaction between asdf and astropy during the test collection process. For example the issue disappears if I manually uninstall asdf-astropy as this turns off all the asdf related tests in astropy, which only consist of the doctests here:

.. doctest-requires:: asdf-astropy
>>> from asdf import AsdfFile
>>> from astropy.modeling import models
>>> rotation = models.Rotation2D(angle=23.7)
>>> f = AsdfFile()
>>> f.tree['model'] = rotation
>>> f.write_to('rotation.asdf')
To read the file and create the model:
.. doctest-requires:: asdf-astropy
>>> import asdf
>>> with asdf.open('rotation.asdf') as f:
... model = f.tree['model']
>>> print(model)
Model: Rotation2D
Inputs: ('x', 'y')
Outputs: ('x', 'y')
Model set size: 1
Parameters:
angle
-----
23.7

As in this case the doctests will be skipped if asdf-astropy is not installed.

This was run on an M3 Mac using Python 3.12 with installed packages:

Package                  Version                Editable project location
------------------------ ---------------------- ----------------------------------------------
aiobotocore              2.12.1
aiohttp                  3.9.3
aioitertools             0.11.0
aiosignal                1.3.1
asdf                     3.1.0
asdf-astropy             0.5.0
asdf-coordinates-schemas 0.2.0
asdf_standard            1.1.0
asdf-transform-schemas   0.4.0
asdf-unit-schemas        0.1.0
astropy                  6.1.dev555+gd32a06ad59 /Users/williambrianjamieson/Workspaces/astropy
astropy-iers-data        0.2024.3.4.0.30.17
asttokens                2.4.1
attrs                    23.2.0
beautifulsoup4           4.12.3
bleach                   6.1.0
botocore                 1.34.51
Bottleneck               1.3.8
certifi                  2024.2.2
cfgv                     3.4.0
click                    8.1.7
cloudpickle              3.0.0
contourpy                1.2.0
coverage                 7.4.3
cycler                   0.12.1
dask                     2024.2.1
decorator                5.1.1
distlib                  0.3.8
execnet                  2.0.2
executing                2.0.1
filelock                 3.13.1
fonttools                4.49.0
frozenlist               1.4.1
fsspec                   2024.2.0
h5py                     3.10.0
html5lib                 1.1
hypothesis               6.98.17
identify                 2.5.35
idna                     3.6
importlib-metadata       7.0.1
iniconfig                2.0.0
ipython                  8.22.2
jedi                     0.19.1
jmespath                 1.0.1
jplephem                 2.21
kiwisolver               1.4.5
locket                   1.0.0
matplotlib               3.8.3
matplotlib-inline        0.1.6
mpmath                   1.3.0
multidict                6.0.5
nodeenv                  1.8.0
numpy                    1.26.4
packaging                23.2
pandas                   2.2.1
parso                    0.8.3
partd                    1.4.1
pexpect                  4.9.0
pillow                   10.2.0
pip                      24.0
platformdirs             4.2.0
pluggy                   1.4.0
pre-commit               3.6.2
prompt-toolkit           3.0.43
ptyprocess               0.7.0
pure-eval                0.2.2
pyarrow                  15.0.0
pyerfa                   2.0.1.1
Pygments                 2.17.2
pyparsing                3.1.1
pytest                   8.0.2
pytest-arraydiff         0.6.1
pytest-astropy           0.11.0
pytest-astropy-header    0.2.2
pytest-cov               4.1.0
pytest-doctestplus       1.2.0
pytest-filter-subpackage 0.2.0
pytest-mock              3.12.0
pytest-remotedata        0.4.1
pytest-xdist             3.5.0
python-dateutil          2.9.0.post0
pytz                     2024.1
PyYAML                   6.0.1
s3fs                     2024.2.0
scipy                    1.12.0
semantic-version         2.10.0
setuptools               69.1.1
six                      1.16.0
sortedcontainers         2.4.0
soupsieve                2.5
stack-data               0.6.3
toolz                    0.12.1
traitlets                5.14.1
typing_extensions        4.10.0
tzdata                   2024.1
urllib3                  2.0.7
virtualenv               20.25.1
wcwidth                  0.2.13
webencodings             0.5.1
wrapt                    1.16.0
yarl                     1.9.4
zipp                     3.17.0
@WilliamJamieson
Copy link
Contributor Author

I suspect this is an interaction between asdf's packaged pytest plugin and pytest-doctestplus.

@pllim pllim added the testing label Mar 6, 2024
@MridulS
Copy link
Contributor

MridulS commented Mar 6, 2024

I can reproduce this issue with a clean build. As yes it does seem like the this is something to do with the important machinery of how asdf is imported.

As import astropy hits the __init__.py files it will import asdf first which will mess up the pytest plugin registry.

To test this just turning off the asdf_astropy import should fix this.

--- a/astropy/table/__init__.py
+++ b/astropy/table/__init__.py
@@ -123,5 +123,5 @@ with registry.delay_doc_updates(Table):

     from .jsviewer import JSViewer

-    if optional_deps.HAS_ASDF_ASTROPY:
-        import asdf_astropy.io.connect
+    #if optional_deps.HAS_ASDF_ASTROPY:
+    #    import asdf_astropy.io.connect

Maybe this issue just means it's time to relook #16152 and if it's worth the maintenance overhead to maintain a pytest wrapper inside astropy.

@pllim
Copy link
Member

pllim commented Mar 6, 2024

@WilliamJamieson , do we still need the commented if lines?

Thanks for investigating, @MridulS !

@WilliamJamieson
Copy link
Contributor Author

I believe we do because that is what enables astropy tables to be backed by ASDF flies.

@pllim
Copy link
Member

pllim commented Mar 6, 2024

This was for backward compatibility, right? I wonder if we can get away by removing it for v7.0 now that astropy.io.misc.asdf is no more.

if optional_deps.HAS_ASDF_ASTROPY:
import asdf_astropy.io.connect

Other packages like specutils register their own stuff (https://github.com/astropy/specutils/blob/main/specutils/io/registers.py), so why can't this be downstream in asdf-astropy now?

@pllim
Copy link
Member

pllim commented Mar 6, 2024

For now, the best low-impact idea I have is to create a "Known Issue" entry to ask people to uninstall asdf-astropy if they must run astropy.test(). 🤷

@pllim pllim changed the title Running tests from python can sometimes fail Running tests from astropy.test() fails when asdf-astropy is installed Mar 6, 2024
@namurphy
Copy link
Contributor

namurphy commented Mar 7, 2024

If the long-term plan is to keep astropy.test(), I'm wondering if it would be worth adding a monthly cron job to make sure it continues to work, at least when asdf-astropy isn't installed. I'd say that if it's important enough to keep as a top-level part of the API, it's important enough to test!

+1 on adding this to the docs as a known issue!

And thank you for raising this issue!

@neutrinoceros
Copy link
Contributor

I wonder if astropy.test() has any value within the packaging community.
I'm genuinely curious what @olebole thinks of maybe removing it ?

@olebole
Copy link
Member

olebole commented Mar 7, 2024

In Debian, we use pytest only (specifically python3 -m pytest --astropy-header -m "not hypothesis" --pyargs astropy for the CI test on the installed package), so astropy.test() can be removed IMO.

@pllim
Copy link
Member

pllim commented Mar 7, 2024

Would a known issue entry like this help?

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

Successfully merging a pull request may close this issue.

6 participants