There is a pytest dependency in the runtime code

[jdavies-st]

Currently the package depends on pytest in the runtime code

specutils/specutils/io/asdf/tags/spectra.py

Line 68 in e2d6127

from astropy.tests.helper import quantity_allclose

but pytest is not an install dependency, only a test dependency. This import from astropy should not be used. Fix should look something like the following

spacetelescope/gwcs#287

[jdavies-st]

More generally, test assertion helpers should not be part of the serialization class. They should be part of the test infrastructure.

Better would be to implement __eq__ for the Spectrum1D class if possible. Though because that class contains quantities, and quantities generally do not have __eq__ this probably has the ripple effect of making __eq__ for Spectrum1D a non-starter.

Comments @eteq @nmearl ?

[dhomeier]

As I recall from the discussion on astropy/astropy#7252 that import is now simply a
from astropy.units import allclose as quantity_allclose
so would this not suffice as replacement? Or for better style perhaps replace all (2) quantity_allclose -> allclose?

[jdavies-st]

Thanks @dhomeier! I didn't know this new function existed.

We've had to deal with this in a bunch of packages, basically any package that includes ASDF functionality. I fixed these in the ASDF types in astropy a while back, but was not aware of this other method existed. This is much cleaner.

The old one in astropy.tests.helper should have been deprecated (that's the whole point of deprecations) to let us know. I see you discussed it in the PR and decided against it.

The old one should be deprecated now I think. Or else this problem will continue to pop up.

[dhomeier]

The old one in astropy.tests.helper should have been deprecated (that's the whole point of deprecations) to let us know. I see you discussed it in the PR and decided against it.

If I read astropy/astropy#7252 (comment) correctly, that was decided because the new interface was never backported to Astropy LTS, to make it easier for affiliated packages to support both versions. @astrofrog - does this still hold even for marking deprecated?