New CircularAnnulusROI class to represent circular annulus ROI #2403
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
astrofrog
requested changes
May 12, 2023
pllim
force-pushed
the
annulus-eclipse-of-my-heart
branch
from
afc53ac
to
fcc436a
Compare
|
Using the example posted above, this is what the polygon looks like: from matplotlib import pyplot as plt
x, y = ann.to_polygon()
plt.gca().set_aspect('equal')
plt.plot(x, y)
|
|
Great! Maybe we should have 100 points instead of 20? |
from 20 for calls in to_polygon involving theta
|
Done!
|
This comment was marked as resolved.
This comment was marked as resolved.
pllim
added a commit
to pllim/glue-astronomy
that referenced
this pull request
May 12, 2023
pllim
added a commit
to pllim/jdaviz
that referenced
this pull request
May 12, 2023
and allow IMPORT DATA to load reg files. Annulus support needs glue-viz/glue#2403 and glue-viz/glue-astronomy#92 [ci skip] [skip rtd]
to polygon
9 tasks
astrofrog
changed the title
pllim
added a commit
to pllim/jdaviz
that referenced
this pull request
Jun 5, 2023
and allow IMPORT DATA to load reg files. Annulus support provided by glue-viz/glue#2403 and glue-viz/glue-astronomy#92 Add test for exception. Bump minimum requirements for glue-core and glue-astronomy.
rosteen
added a commit
to spacetelescope/jdaviz
that referenced
this pull request
Jun 13, 2023
* Added .ico and .svg files * app-wide display unit in specviz (#2048) * basic app-wide display unit in specviz (not all plugins are updated yet) * unit-aware select component which maps to glue-supported unit strings * implement use_display_units option for get_data, get_subsets Co-authored-by: Jesse Averbukh <javerbukh@gmail.com> Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> * bump glue-core to necessary version for display units * change PR number in changelog to main PR * Display units line analysis (#2128) * emit event when display unit changed * update line analysis for unit changes * cubeviz/slice plugin unit conversion support (we might want to generalize some of this logic more into the mark's _update_data when adding support for spectral lines as well) * Display units support for markers plugin (#2130) * update markers plugin for display units * move unit-updating logic to mark itself * LineUncertainties to be unit-aware * fix support for markers in cubeviz * display units: fix failing tests (#2132) * fix error raised when spectrum uncertainty is None * fix RTD build failure because of missing mixin available in import * fix test setting display unit now that um is preferred to micron * add disabled unit conversion plugin to mosviz * avoid unit-conversion error in cubeviz tests * bump versions of glue/glue-jupyter * fix deg intialization in cubeviz slice plugin * add LinesAutoUnit to __all__ so RTD can find it * add use_display_units kwarg to get_spectral_regions * fix setting display units for unitless data * Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * Display units: line lists (#2148) * BaseSpectrumVerticalLine to inherit unit-support from PluginMark * convert units on internal rest value * code cleanup * remove unit conversion plugin from all bug specviz * for initial implementation - eventually we'll want to implement across all plugins/viewers * get_display_unit fallback for configs without unit conversion plugin * Update glue-core dependency * Update Subset Tools plugin to use display units (#2195) * bump glue-core to necessary version for display units * cubeviz/slice plugin unit conversion support (we might want to generalize some of this logic more into the mark's _update_data when adding support for spectral lines as well) * display units: fix failing tests (#2132) * fix error raised when spectrum uncertainty is None * fix RTD build failure because of missing mixin available in import * fix test setting display unit now that um is preferred to micron * add disabled unit conversion plugin to mosviz * avoid unit-conversion error in cubeviz tests * bump versions of glue/glue-jupyter * fix deg intialization in cubeviz slice plugin * add LinesAutoUnit to __all__ so RTD can find it * add use_display_units kwarg to get_spectral_regions * fix setting display units for unitless data * Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * remove unit conversion plugin from all bug specviz * for initial implementation - eventually we'll want to implement across all plugins/viewers * Working on unit conversion handling in subset plugin * Get subset updating working in non-default units * Add changelog * Update jdaviz/configs/default/plugins/subset_plugin/subset_plugin.vue Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> --------- Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * allow downstream apps to set what classes are considered native marks * Fix unit conversion for PluginMarks and y-limits (#2215) * Fix unit conversion for PluginMarks and y-limits * Only use equivalency if it's a spec viewer * Store wavelength unit on data update * Remove redundant line * Better check in marks * Replace subset_to_apply in docs where possible (#2223) * Replace subset_to_apply in docs where possible (#2223) * Avoid trying to convert units of empty marks arrays * Add open method to automatically detect config and load data * Update demo notebooks * Codestyle * Add autoconfig test coverage * Revert "Update demo notebooks" This reverts commit cc513b3. * Add mention of open to relevant example notebooks * Prepare to support open via cli * Remove the need to open file twice for autoconfig detection * Fix Helper vs App Docstring * TST: Change PIP_EXTRA_INDEX_URL because packages moved where they upload nightly builds. * Display units: model fitting (#2216) * model component compatibility based on display units * equation validity based on display units * allow re-estimating model parameters to update compat checks * test coverage * get_data(use_display_units) to pass spectral_density equivalency * handle whitespace in model equation * Add clarity for kwargs * plugin results: support overwriting when data loaded in multiple viewers (#2222) * reverts removal of call to set_plot_axes when loading data into viewer (#2235) * fixes a bug in the axes labels on cube viewers in cubeviz as well as setting the correct attributes in lcviz * Subsume failing identifier test into autoconfig test * Update subset plugin UI (#2234) * Update subset plugin UI * Apply suggestions from code review Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> --------- Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> * Changelog * Add docs page to explain how to create jdaviz-readable products (#2228) * Add page for data products * Update index * Fix title * Capital letters * Address first review comments * Edit format * Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * Including reference in import_data * Update docs/create_products.rst * Update docs/create_products.rst --------- Co-authored-by: Camilla Pacifici <cpacifici@stsci.edu> Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> Co-authored-by: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> * FEAT: Load annulus from file and allow IMPORT DATA to load reg files. Annulus support provided by glue-viz/glue#2403 and glue-viz/glue-astronomy#92 Add test for exception. Bump minimum requirements for glue-core and glue-astronomy. * disallow gaussian smooth output as input * BUG: Fix ellipse translation in app.get_subsets() * DOC: Use pydata-sphinx-theme (#2243) * DOC: Use pydata-sphinx-theme that has dark mode, modern design, and mobile friendly DOC: No longer need tomli DOC: Now requires sphinx-astropy 1.9.1 * Add cards to index page with icons * Remove iframe hardcoding so it scales in mobile mode * DOC: Insert small logo next to title instead of a giant logo above the title * Add app method to simplify spectral subset (#2237) * Add app method to simplify spectral subset Add simplify button to subset plugin * Fix xor bug exposed by simplify button * Fix style checks * Remove print statements * Add tooltip to simplify button * Make button appear only if the subset can be simplified * Make simplify button appear only when applicable * Apply suggestions from code review Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * Update changes file --------- Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> * Fix missed conflict --------- Co-authored-by: Jennifer Kotler <jkotler@esme.stsci.edu> Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> Co-authored-by: Jesse Averbukh <javerbukh@gmail.com> Co-authored-by: Ricky O'Steen <rosteen@stsci.edu> Co-authored-by: Ricky O'Steen <39831871+rosteen@users.noreply.github.com> Co-authored-by: Duy Nguyen <duytnguyendtn.open@gmail.com> Co-authored-by: Camilla Pacifici <camilla.pacifici@gmail.com> Co-authored-by: Camilla Pacifici <cpacifici@stsci.edu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new class to represent circular annulus as ROI.
Motivation: Even though we can use annulus now after glue-viz/glue-astronomy#90 , it is not very user-friendly, as we have to expose parameters from the outer and inner circles separately, which would cause user to easily make the subset state no longer a circular annulus (see spacetelescope/jdaviz#2182 (comment) for exciting screenshots). Therefore, there is a need to represent this shape as ROI so that we only expose the relevant parameters (one center only, inner radius, outer radius) without a lot of hacky workaround downstream.
With this patch, I could do the following in Jdaviz after unpinning maxversion of glue-core (xref spacetelescope/jdaviz#2183).
The next step following this PR is to support this new ROI both in glue-astronomy (glue-viz/glue-astronomy#92) and Jdaviz.
🐱