astropy / package-template

Template for packages that use Astropy. Maintainer: @astrofrog
http://docs.astropy.org/projects/package-template/en/latest/
Other
60 stars 63 forks source link

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

Closed stargaser closed 4 years ago

stargaser commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@tcassanelli , does this help? https://github.com/astropy/pytest-astropy-header#migrating-from-the-astropy-header-plugin-to-pytest-astropy

tcassanelli commented 4 years ago

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 commented 4 years ago

I tracked the deprecation warning down to a leftover reference to the plugin in astropy core, which I've fixed in https://github.com/astropy/astropy/pull/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 commented 4 years ago

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

tcassanelli commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

Fixed in #438