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 12 forks source link

Add support for elliptical ROI #32

Closed javerbukh closed 3 years ago

javerbukh commented 3 years ago

Pull Request Template

Description

Potentially fixes #31

I'm not sure if anything else needs to be done here to implement elliptical ROI in glue-astronomy, and by extension jdaviz. Testing on my machine does not look different but I'm not sure if further changes need to be made in glue-jupyter or jdaviz.

codecov[bot] commented 3 years ago

Codecov Report

Merging #32 (5376944) into master (930fd84) will decrease coverage by 0.86%. The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #32      +/-   ##
==========================================
- Coverage   97.27%   96.41%   -0.87%     
==========================================
  Files          13       15       +2     
  Lines         991     1003      +12     
==========================================
+ Hits          964      967       +3     
- Misses         27       36       +9     
Impacted Files Coverage Δ
glue_astronomy/translators/regions.py 95.32% <50.00%> (-0.87%) :arrow_down:
...onomy/io/spectral_cube/tests/test_spectral_cube.py 85.71% <0.00%> (-14.29%) :arrow_down:
glue_astronomy/conftest.py 100.00% <0.00%> (ø)
glue_astronomy/__init__.py 71.42% <0.00%> (ø)

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 930fd84...5376944. Read the comment docs.

pllim commented 3 years ago

I also wonder how this will affect spacetelescope/jdaviz#697 🤔

eteq commented 3 years ago

(also that "build not found" error is weird, but will troubleshoot with @rosteen out-of-band) EDIT: seems to have been a one-off, but the real problem I'm trying to troubleshoot in #34

pllim commented 3 years ago

It's passing on windows though, so I'm pretty confident there's not a concern there.

See, Windows is useful sometimes. 😆