Closed marcelo-alvarez closed 1 year ago
@dkirkby or @weaverba137 do you have experience porting a package away from astropy.tests.plugins
? I'm not excited about the solution suggested at https://github.com/astropy/pytest-astropy-header which seems to suggest installing another custom package. How would you feel about de-astropyifying the specsim unit test infrastructure?
This is a blocking factor for using astropy 5.2, which was an optional update that we were hoping to include in the DR1/Iron software environment.
It depends on how far you want to go.
In this particular instance, astropy.tests.plugins.display
, which is now in a separate package, was providing some additional header information for the unit tests, but not affecting the unit tests themselves.
Do you want to take the trivial step of removing this header information, or do you want to purge all Astropy infrastructure from specsim?
Historically, specsim was meant to be an Astropy-affiliated package, and there is an expectation that Astropy-affiliated packages use the Astropy infrastructure. Are we considering re-evaluating that position?
@marcelo-alvarez , is there a particular branch you are working on?
Let me emphasize: I am not actually advocating in favor of removing Astropy infrastructure. On the contrary, I think this is a trivial fix: just don't display those headers, they aren't needed for the actual tests.
But I do want to move forward the discussion of the long-term future of the specsim package, and who is maintaining it.
One thing I will note is that unit tests of this package are going to be difficult until we migrate to GitHub Actions. Until then we will need more details about the exact environment that @marcelo-alvarez is using that has Astropy/5.2.
PR #115 is for migrating to github actions. The blocking factor might be a maintainer who feels ownership of approving PRs and making these changes.
@weaverba137 good point about astropy-affiliated packages using astropy infrastructure, as long as we have a maintainer keeping up with that infrastructure. For now I'm happy with whatever is the easiest solution to restore unit tests for astropy 5.2 while remaining backwards compatibility with 5.0.
I saw that, but we would need to be careful because PR #115 doesn't actually include tests on Astropy/5.2. so additional work would be needed to sync that with the environment that @marcelo-alvarez is using.
For the immediate fix of the unit test issue I will self assign and work on it. It sounds like the preferred solution here is just to not display the headers in the tests moving forward.
@dylanagreen, thanks, indeed it looked like it would be a simple fix and glad it turned out to be.
One thing I will note is that unit tests of this package are going to be difficult until we migrate to GitHub Actions. Until then we will need more details about the exact environment that @marcelo-alvarez is using that has Astropy/5.2.
@weaverba137 in case this is still useful to you in some larger context than resolving this issue, you can obtain the environment I used by typing
source /global/common/software/desi/users/malvarez/desi_environment.sh local 2.0.1
on a Perlmutter login node.
fails with
when using astropy/5.2. See https://github.com/astropy/pytest-astropy-header for a possible solution.
@sbailey