glue-viz / glue-astronomy

Plugin to add astronomy-specific functionality to glue
https://glue-astronomy.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
11 stars 13 forks source link

Change Spectrum1DHandler default stat back to mean #40

Closed pllim closed 3 years ago

pllim commented 3 years ago

Description

This is my attempt to fix spacetelescope/jdaviz#821 . With this patch, the code snippet posted in spacetelescope/jdaviz#821 went from traceback to this:

>>> spec = data.get_object(cls=Spectrum1D) 
<Spectrum1D(flux=<Quantity [1., 1., 1.]>, spectral_axis=<SpectralAxis
   (observer to target:
      radial_velocity=0.0 km / s
      redshift=0.0)
  [1., 2., 3.] Hz>)>

I don't grok why the default was changed in #36 , so @rosteen or @astrofrog can comment.

codecov[bot] commented 3 years ago

Codecov Report

Merging #40 (0968b6c) into main (03e439a) will decrease coverage by 0.09%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #40      +/-   ##
==========================================
- Coverage   95.93%   95.84%   -0.10%     
==========================================
  Files          15       15              
  Lines        1034     1034              
==========================================
- Hits          992      991       -1     
- Misses         42       43       +1     
Impacted Files Coverage Δ
glue_astronomy/translators/spectrum1d.py 96.42% <100.00%> (-1.20%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 03e439a...0968b6c. Read the comment docs.

pllim commented 3 years ago

This failure might be related but I don't know what it is about:

_________________________ test_from_spectrum1d[wcs3d] _________________________

mode = 'wcs3d'

    @pytest.mark.parametrize('mode', ('wcs1d', 'wcs3d', 'lookup'))
    def test_from_spectrum1d(mode):

        if mode == 'wcs3d':
            # This test is intended to be run with the version of Spectrum1D based
            # on NDCube 2.0
            pytest.importorskip("ndcube", minversion="1.99")

            # Set up simple spatial+spectral WCS
            wcs = WCS(naxis=3)
            wcs.wcs.ctype = ['RA---TAN', 'DEC--TAN', 'FREQ']
            wcs.wcs.set()
            flux = np.ones((4, 4, 5))*u.Unit('Jy')
            uncertainty = VarianceUncertainty(np.square(flux*0.1))
            mask = np.zeros((4, 4, 5))
            kwargs = {'wcs': wcs, 'uncertainty': uncertainty, 'mask': mask}
        else:

...

        spec = Spectrum1D(flux, **kwargs)

        data_collection = DataCollection()

        data_collection['spectrum'] = spec

        data = data_collection['spectrum']

        assert isinstance(data, Data)
        assert len(data.main_components) == 3
        assert data.main_components[0].label == 'flux'
>       assert_allclose(data['flux'], flux.value)
E       AssertionError: 
E       Not equal to tolerance rtol=1e-07, atol=0
E       
E       (shapes (4, 5, 4), (4, 4, 5) mismatch)
E        x: array([[[1., 1., 1., 1.],
E               [1., 1., 1., 1.],
E               [1., 1., 1., 1.],...
E        y: array([[[1., 1., 1., 1., 1.],
E               [1., 1., 1., 1., 1.],
E               [1., 1., 1., 1., 1.],...

glue_astronomy\translators\tests\test_spectrum1d.py:166: AssertionError
astrofrog commented 3 years ago

I'll defer to @rosteen since he wrote the 3D handler in this code.

astrofrog commented 3 years ago

The fix here makes sense because Spectrum1D has issues getting initialised with n-d arrays: https://github.com/astropy/specutils/issues/857

astrofrog commented 3 years ago

I see the failure in another PR so merging this as I think it's unrelated