Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract SpectralRegions from specviz app #161

Merged
merged 4 commits into from
Jun 26, 2020

Conversation

nmearl
Copy link
Contributor

@nmearl nmearl commented Jun 17, 2020

This PR allows for extracting subsets as SpectralRegion objects from the specviz app. Requires #159.

@nmearl nmearl requested a review from eteq June 18, 2020 14:05
@nmearl nmearl force-pushed the feat/specviz-region-extract branch 2 times, most recently from 39cd4d2 to 200a9b0 Compare June 22, 2020 14:09
Copy link
Contributor

@eteq eteq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broadly good, but a couple minor inline items. Note thought this collides with the changes in #165 so will something is going to need a rebase...

jdaviz/helpers.py Outdated Show resolved Hide resolved
jdaviz/helpers.py Outdated Show resolved Hide resolved
jdaviz/helpers.py Outdated Show resolved Hide resolved
@nmearl nmearl force-pushed the feat/specviz-region-extract branch from cce142e to d938651 Compare June 26, 2020 15:59
@nmearl nmearl mentioned this pull request Jun 26, 2020
Copy link
Contributor

@eteq eteq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like some of my comments above got mixed up in the rebase, but some of it still applies.

jdaviz/configs/specviz/helper.py Outdated Show resolved Hide resolved
jdaviz/configs/specviz/helper.py Outdated Show resolved Hide resolved
@eteq eteq merged commit 9269a46 into spacetelescope:master Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants