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.

Sign up for GitHub

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

Hide Cubeviz import button after importing data #1495

Merged
merged 5 commits into from
Jul 19, 2022
Merged

Hide Cubeviz import button after importing data #1495

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1a14958
Hid Cubeviz import button after importing data
ojustino Jul 14, 2022
1c28ea0
Allowed kwargs in Cubeviz load_data method
ojustino Jul 15, 2022
9afb1c2
Updated changelog
ojustino Jul 15, 2022
94e9d62
Test multiple imports in Cubeviz
ojustino Jul 15, 2022
2489839
Updated 'Import Data' card text for Cubeviz in Vue
ojustino Jul 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ New Features
Cubeviz
^^^^^^^

- The "Import Data" button is hidden after a data cube is loaded into the app [#1495]

Imviz
^^^^^

Expand Down Expand Up @@ -79,7 +81,7 @@ Other Changes and Additions

New Features
------------
- The app and individual plugins can be opened in a new window by clicking a button in the top
- The app and individual plugins can be opened in a new window by clicking a button in the top
right-hand corner. [#977, #1423]

- Snackbar queue priority and history access. [#1352, #1437]
Expand Down Expand Up @@ -126,7 +128,7 @@ API Changes
have been renamed to "Colormap" and "Monochromatic," respectively, for all image
viewers. [#1406]

- Viz tool display changed to ``viz.show()`` from ``viz.app``. Sidecar no longer returned by
- Viz tool display changed to ``viz.show()`` from ``viz.app``. Sidecar no longer returned by
show methods. [#965]

Imviz
Expand Down Expand Up @@ -188,7 +190,7 @@ Imviz
Mosviz
^^^^^^

- Data dropdown in the gaussian smooth plugin is limited to data entries from the
- Data dropdown in the gaussian smooth plugin is limited to data entries from the
spectrum-viewer (excluding images and 2d spectra). [#1452]

2.6 (2022-05-25)
Expand Down Expand Up @@ -328,7 +330,7 @@ Other Changes and Additions
- Help button in toolbar to open docs in a new tab. [#1240]
- Snackbar queue handles loading interrupt more cleanly. [#1249]
- Reported quantities are rounded/truncated to avoid showing unnecessary precision. [#1244]
- Line analysis quantities are coerced so length units cancel and constants are removed from units.
- Line analysis quantities are coerced so length units cancel and constants are removed from units.
[#1261]

2.4 (2022-03-29)
Expand Down Expand Up @@ -371,7 +373,7 @@ Bug Fixes
---------

- Fixed support for table scrolling by enabling scrollbar. [#1116]
- Fixed loading additional spectra into a spectrum viewer after creating a
- Fixed loading additional spectra into a spectrum viewer after creating a
spectral subset. [#1205]

Cubeviz
Expand Down Expand Up @@ -404,7 +406,7 @@ Other Changes and Additions
- Viewer toolbars are now nested and consolidated, with viewer and layer options
moved to the sidebar. [#1140]

- Redshifts imported with a custom line list are now ignored. Redshift must be set app-wide via
- Redshifts imported with a custom line list are now ignored. Redshift must be set app-wide via
viz.set_redshift or the line list plugin. [#1134]

- Subset selection dropdowns in plugins now show synced color indicators. [#1156, #1175]
Expand Down Expand Up @@ -532,7 +534,7 @@ Specviz
Other Changes and Additions
---------------------------

- Redshift slider and options are moved from the toolbar to the Line List
- Redshift slider and options are moved from the toolbar to the Line List
plugin in the plugin tray. [#1031]

- Spectral lines and redshift are refactored to improve performance. [#1036]
Expand Down Expand Up @@ -586,7 +588,7 @@ Imviz
Mosviz
^^^^^^

- New toggle button to lock/unlock viewer settings (x-limits in 1d and 2d spectrum viewers and
- New toggle button to lock/unlock viewer settings (x-limits in 1d and 2d spectrum viewers and
stretch and percentile for 2d spectrum and image viewers). [#918]

- Ability to add custom columns and change visibility of columns in the table. [#961]
Expand Down
15 changes: 8 additions & 7 deletions jdaviz/app.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@
'padding-left': '15%',
'padding-top': '20px'}"
@click="state.logger_overlay = false">
<v-row
dense
<v-row
dense
@click="(e) => {e.stopImmediatePropagation()}"
v-for="history in state.snackbar_history.slice().reverse()"
style="width: 80%">
<v-alert
dense
:type="history.color"
<v-alert
dense
:type="history.color"
style="width: 100%; margin: 6px 0px 0px; text-align: left">
[{{history.time}}]: {{history.text}}
</v-alert>
Expand All @@ -38,7 +38,8 @@
<v-toolbar-items v-for="item in state.tool_items">
<v-divider v-if="['g-data-tools', 'g-subset-tools'].indexOf(item.name) === -1" vertical style="margin: 0px 10px"></v-divider>
<v-divider v-else-if="item.name === 'g-subset-tools'" vertical style="margin: 0px 10px; border-width: 0"></v-divider>
<j-tooltip :tipid="item.name">
<j-tooltip v-if="config == 'cubeviz' && item.name == 'g-data-tools' && state.data_items.length !== 0"></j-tooltip>
<j-tooltip v-else :tipid="item.name">
Comment on lines +41 to +42
Copy link
Member

Choose a reason for hiding this comment

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

could probably either make the first (empty) v-if entry a div instead of a j-tooltip and/or invert the v-if and not render any entry in the case of hiding the import button (but that might be harder logic to parse).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Giving both the same element seemed more intuitive to me. I did want to put the conditional for hiding in the v-else statement since it's rarer, but the logic for the v-if seems like it would need to be long – I get something like v-if="(config !== 'cubeviz') || ((config == 'cubeviz') && (item.name !== 'g-data-tools' || (item.name == 'g-data-tools' && state.data_items.length == 0)))".

<jupyter-widget :widget="item.widget" :key="item.name"></jupyter-widget>
</j-tooltip>
</v-toolbar-items>
Expand Down Expand Up @@ -375,7 +376,7 @@ a:active {

.color-to-accent {
/* https://codepen.io/sosuke/pen/Pjoqqp for #C75109 */
filter: brightness(0) saturate(100%) invert(31%) sepia(84%) saturate(1402%) hue-rotate(1deg) brightness(95%) contrast(94%);
filter: brightness(0) saturate(100%) invert(31%) sepia(84%) saturate(1402%) hue-rotate(1deg) brightness(95%) contrast(94%);
}

.v-overlay__content {
Expand Down
16 changes: 16 additions & 0 deletions jdaviz/configs/cubeviz/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,22 @@ def _set_spectrum_x_axis(self, msg):
else:
viewer.state.x_att_pixel = ref_data.id["Pixel Axis 2 [x]"]

def load_data(self, data, **kwargs):
"""
Load and parse a data cube with Cubeviz.
(Note that only one cube may be loaded per Cubeviz instance.)

Parameters
----------
data : str or `~astropy.io.fits.HDUList`
A string file path or astropy FITS object pointing to the
data cube.
"""
if len(self.app.state.data_items) != 0:
raise RuntimeError('only one cube may be loaded per Cubeviz instance')
Comment on lines +52 to +53
Copy link
Member

Choose a reason for hiding this comment

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

just realized that some day we might want to allow loading 1d spectra and only forbid cubes, but that will require quite a bit more advanced logic, especially on the UI side, so I think this is fine for now until/unless we get a request to re-enable that.


super().load_data(data, parser_reference="cubeviz-data-parser", **kwargs)

def select_slice(self, slice):
"""
Select a slice by index.
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/configs/cubeviz/plugins/parsers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def parse_data(app, file_obj, data_type=None, data_label=None):
----------
app : `~jdaviz.app.Application`
The application-level object used to reference the viewers.
file_path : str
file_obj : str
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not quite groking this change here; the docstring still explicitly states "path" , so presumably if this change is to explicitly allowing a Spectrum1D to be loaded directly, then I think the docstring should be updated too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mainly did this for consistency – if you search, file_path doesn't appear anywhere else in the file. I'm guessing the argument was originally named file_path and was later renamed as file_obj without a corresponding update to the docstring. I don't do anything to introduce support for Spectrum1D in this PR. Are you saying that happened elsewhere and should have been mentioned in the docstring?

The path to a cube-like data file.
data_type : str, {'flux', 'mask', 'uncert'}
The data type used to explicitly differentiate parsed data.
Expand Down
8 changes: 8 additions & 0 deletions jdaviz/configs/cubeviz/plugins/tests/test_parsers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,11 @@ def test_spectrum1d_parse(spectrum1d, cubeviz_helper):
def test_numpy_cube(cubeviz_helper):
with pytest.raises(NotImplementedError, match='Unsupported data format'):
cubeviz_helper.load_data(np.ones(27).reshape((3, 3, 3)))


def test_multiple_load(image_hdu_obj, cubeviz_helper):
cubeviz_helper.load_data(image_hdu_obj)

# first load should be succesful; subsequent attempts should fail
with pytest.raises(RuntimeError, match=r".?only one (data)?cube.?"):
cubeviz_helper.load_data(image_hdu_obj)
2 changes: 1 addition & 1 deletion jdaviz/configs/mosviz/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ def load_data(self, spectra_1d=None, spectra_2d=None, images=None,
spectra_1d_label=None, spectra_2d_label=None,
images_label=None, *args, **kwargs):
"""
Load and parse a set of MOS spectra and images
Load and parse a set of MOS spectra and images.

Parameters
----------
Expand Down