Skip to content
This repository has been archived by the owner on Aug 11, 2023. It is now read-only.

Handling deprecation of astropy.tests.plugins.display items #435

Closed
stargaser opened this issue Oct 29, 2019 · 13 comments
Closed

Handling deprecation of astropy.tests.plugins.display items #435

stargaser opened this issue Oct 29, 2019 · 13 comments

Comments

@stargaser
Copy link

In the course of using the package template, we found some tests against the development version of astropy failing due to deprecation of items in astropy.tests.plugins.display. In particular, in conftest.py there is:

    from astropy.tests.plugins.display import (
                pytest_report_header,
                PYTEST_HEADER_MODULES,
                TESTED_VERSIONS)

The import is successful in astropy 3.2.3 but fails in the development version.

The fix is to instead import these items from pytest_astropy_header.display. Does this mean that the package pytest_astropy_header should be required for the package template? How do we handle the tests against earlier versions of astropy?

For reference, #this PR from our new package has the solution that we implemented for the time being, which is to catch an ImportError from the import from pytest_astropy_header and then import from astropy.tests.plugins.display in the except clause.

@bsipocz
Copy link
Member

bsipocz commented Oct 29, 2019

Yes, there has been some refactoring in preparation for astropy 4.0, and the changes hasn't yet been propagated all the way downstream.

@astrofrog
Copy link
Member

Hmm I didn't consider that the deprecation warnings would end up causing CI failures. We could always remove the deprecation warning for a transition period?

@bsipocz
Copy link
Member

bsipocz commented Oct 29, 2019

I think the real issue is that pytest_report_header has ceased to be as now we're registering the plugin via the pytest config mechanism, and thus it's failing for every package that tries to import it from astropy.

As for deprecations, I think that packages should fix those, that's why we're raising them.

@tcassanelli
Copy link

tcassanelli commented Nov 5, 2019

I updated .travis.yml, setup.cfg and conftest.py as suggested in #436, but I keep getting the following error in the developer version

=============================== warnings summary ===============================
2094/home/travis/miniconda/envs/test/lib/python3.6/site-packages/_pytest/config/__init__.py:830
2095  /home/travis/miniconda/envs/test/lib/python3.6/site-packages/_pytest/config/__init__.py:830: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: pytest_astropy_header
2096    self._mark_plugins_for_rewrite(hook)

WARNING: AstropyDeprecationWarning: The astropy.tests.plugins.display plugin has been deprecated. See the pytest-astropy-header documentation for information on migrating to using pytest-astropy-header to customize the pytest header. [astropy.tests.plugins.display]

while running travis

ASTROPY_VERSION=development EVENT_TYPE='pull_request push cron'

@pllim
Copy link
Member

pllim commented Nov 5, 2019

@tcassanelli , are you doing a from astropy.tests.plugins import * or something like that in your package? I might have also missed something in #436, so please let us know if you found a solution.

@tcassanelli
Copy link

@pllim I'm still trying to debug the problem, so far I tried several combinations in the conftest.py file. Do you mean to add the line from astropy.tests.plugins import * in contest.py right?
If I do so I get this error:

Installing the dependency astropy with conda was unsuccessful, using pip instead.
1854ERROR: Invalid requirement: 'astropystable.*'

I'm using the conftest.py package template example of it and I have tried using the one suggested in pytest-astropy-header, but no success.

I have also tried adding pytest_astropy_header to pip-requirements, maybe some other file that I need to add this line?

@pllim
Copy link
Member

pllim commented Nov 5, 2019

@tcassanelli
Copy link

it doesn't I tried it. First I get an issue with pytest_report_header. I erase that name and I still can't get it to work.

@astrofrog
Copy link
Member

I tracked the deprecation warning down to a leftover reference to the plugin in astropy core, which I've fixed in astropy/astropy#9539 - with that and using the recommended conftest.py from the pytest-astropy-header README:

import os

from astropy.version import version as astropy_version
if astropy_version < '3.0':
    from astropy.tests.pytest_plugins import *
    del pytest_report_header
else:
    from pytest_astropy_header.display import PYTEST_HEADER_MODULES, TESTED_VERSIONS


def pytest_configure(config):

    config.option.astropy_header = True

    PYTEST_HEADER_MODULES.pop('Pandas', None)
    PYTEST_HEADER_MODULES['scikit-image'] = 'skimage'

    from .version import version, astropy_helpers_version
    packagename = os.path.basename(os.path.dirname(__file__))
    TESTED_VERSIONS[packagename] = version
    TESTED_VERSIONS['astropy_helpers'] = astropy_helpers_version

Everything works fine for me. @tcassanelli - I'll let you know once the astropy PR is merged and if it still doesn't work for you after that, can you push your changes to a branch and send me a link so I can take a look?

@pllim
Copy link
Member

pllim commented Nov 6, 2019

@tcassanelli , astropy/astropy#9539 is merged. FYI. Your CI should now pick up development version of Astropy with that patch.

@tcassanelli
Copy link

Thanks @astrofrog, @pllim , now I am having another error in the developer version. Everything else builds fine in travis CI. This is the error:

/home/travis/miniconda/envs/test/lib/python3.6/site-packages/astropy/units/quantity.py:1774: UnitsError

This is related to from astropy.tests.helper import assert_quantity_allclose, tests are correct but not the units, which is weird because it only fails while in:

env: ASTROPY_VERSION=development
     EVENT_TYPE='pull_request push cron'

@pllim
Copy link
Member

pllim commented Nov 6, 2019

@tcassanelli , that failure is out of scope as far as the template (this repo) is concerned. If you have astropy dev installed locally, you can see if you can reproduce it. If so, please open an issue at https://github.com/astropy/astropy/issues with a minimal snippet to reproduce the failure. Thanks!

@pllim
Copy link
Member

pllim commented Jan 31, 2020

Fixed in #438

@pllim pllim closed this as completed Jan 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants