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

silx view: Added --slices option #3860

Merged
merged 9 commits into from
Jul 20, 2023
Merged

Conversation

t20100
Copy link
Member

@t20100 t20100 commented Jun 15, 2023

This PR adds the option to pass multiple integers for displaying slices with silx view through a --slice option.

To do so, it adds a DatasetSlice class to expose the opened slice as a dataset and makes the necessary changes to make it work.

The content of this PR is adapted from PR #3677 (which was hard to rebase).
It is a simpler version of PR #3677 since it only provides --slice and not URL #fragment support which could be added later if needed.

@t20100 t20100 added this to the 2.0.0 milestone Jun 15, 2023
@t20100 t20100 requested review from vallsv and payno June 15, 2023 09:09
Comment on lines +65 to +66
if isinstance(h5obj, DatasetSlice):
label += str(list(h5obj.indices))
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not a fan of having specific code for DatasetSlice here, but I couldn't find a better way, except maybe adding this in the silx.io.commonh5 API...

Comment on lines +582 to +583
if isinstance(item, DatasetSlice):
info.append(item.indices)
Copy link
Member Author

Choose a reason for hiding this comment

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

Same, it'd be best to avoid specific code here.
I wanted to rework the way to check equality, but that's complex changes

@t20100
Copy link
Member Author

t20100 commented Jun 15, 2023

CI fails a lot on Windows with python3.7 and pyside6, with exceptions like:

    def tearDown(self):
        self.qapp.processEvents()
    
        if len(self.__class__._exceptions) > 0:
            messages = "\n".join(self.__class__._exceptions)
>           raise AssertionError("Exception occured in Qt thread:\n" + messages)
E           AssertionError: Exception occured in Qt thread:
E           Traceback (most recent call last):
E             File "C:\projects\silx\venv_test\lib\site-packages\silx\gui\data\DataViewer.py", line 290, in __setDataInView
E               if (self.__previousSelection.slice != self.__displayedSelection.slice or
E           AttributeError: 'NoneType' object has no attribute 'slice'
E           Traceback (most recent call last):
E             File "C:\projects\silx\venv_test\lib\site-packages\silx\gui\data\DataViewer.py", line 290, in __setDataInView
E               if (self.__previousSelection.slice != self.__displayedSelection.slice or
E           AttributeError: 'NoneType' object has no attribute 'slice'
E           Traceback (most recent call last):
E             File "C:\projects\silx\venv_test\lib\site-packages\silx\gui\data\DataViewer.py", line 290, in __setDataInView
E               if (self.__previousSelection.slice != self.__displayedSelection.slice or
E           AttributeError: 'NoneType' object has no attribute 'slice'
venv_test\lib\site-packages\silx\gui\utils\testutils.py:188: AssertionError
    def tearDown(self):
        self.qapp.processEvents()
    
        if len(self.__class__._exceptions) > 0:
            messages = "\n".join(self.__class__._exceptions)
>           raise AssertionError("Exception occured in Qt thread:\n" + messages)
E           AssertionError: Exception occured in Qt thread:
E           Traceback (most recent call last):
E             File "C:\projects\silx\venv_test\lib\site-packages\silx\gui\data\DataViewer.py", line 287, in __setDataInView
E               self.__currentView.setData(self.__displayedData)
E             File "C:\projects\silx\venv_test\lib\site-packages\silx\gui\data\DataViews.py", line 657, in setData
E               self.__updateDisplayedView()
E             File "C:\projects\silx\venv_test\lib\site-packages\silx\gui\data\DataViews.py", line 635, in __updateDisplayedView
E               w = self.__currentView.getWidget()
E             File "C:\projects\silx\venv_test\lib\site-packages\silx\gui\data\DataViews.py", line 351, in getWidget
E               self.__widget = self.createWidget(self.__parent)
E             File "C:\projects\silx\venv_test\lib\site-packages\silx\gui\data\DataViews.py", line 1347, in createWidget
E               widget = RecordTableView(parent)
E           AttributeError: __init__
venv_test\lib\site-packages\silx\gui\utils\testutils.py:188: AssertionError

@t20100 t20100 marked this pull request as draft June 15, 2023 11:28
type=int,
nargs='+',
help='List of slice indices to open (Only for dataset)',
)
Copy link
Contributor

@vallsv vallsv Jun 16, 2023

Choose a reason for hiding this comment

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

I think it would be better to use a real slice here

--slice 1,2,3

instead of

--slice 1 2 3

As we already have the parser from the DataUrl, it's costless, and it will support stuffs like 1::2,:10,...

Copy link
Member Author

@t20100 t20100 Jun 16, 2023

Choose a reason for hiding this comment

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

--slice 1 2 3 opens not just one slice (data[1,2,3]) as a dataset but 3 slices as 3 different datasets: data[1], data[2], data[3].
Maybe best to call it --slices.

I haven't done it, but we could also support --slice 1::4,:10,...,1 3:5,:,1 if needed.... but the current main need is to open slices in a 3D stack.

Copy link
Member

@payno payno left a comment

Choose a reason for hiding this comment

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

work smoothly on my datasets

@t20100 t20100 changed the title silx view: Added --slice option silx view: Added --slices option Jul 13, 2023
@t20100 t20100 marked this pull request as ready for review July 13, 2023 15:11
@t20100
Copy link
Member Author

t20100 commented Jul 13, 2023

  • Fixed CI: Updated python used to test with pyside6 in order not to restrict matplotlib version.
  • Changed option from --slice to --slices

Finally ready for review/merge

@t20100 t20100 requested a review from payno July 18, 2023 13:49
Copy link
Member

@payno payno left a comment

Choose a reason for hiding this comment

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

still fine with me.
Maybe we could add an example on how to use it on the PR and on the text documentation (view.rst/ Examples of usage) as well like:

# open specific dataset
silx view 5.06_crayon_W150_60_Al2_W0.25_xc1000_.nx::entry/instrument/detector/data
# open specific set of dataset from name / location
silx view 5.06_crayon_W150_60_Al2_W0.25_xc1000_.nx::entry/*/data
# open specific set of datasets slices (first and last on this example)
silx view 5.06_crayon_W150_60_Al2_W0.25_xc1000_.nx::entry/*/data --slices 0 -1

@t20100
Copy link
Member Author

t20100 commented Jul 20, 2023

I added your examples (reworked a bit) to the doc, including

silx view my_hdf5_file.h5::entry/*/data --slices 0 -1

I also fixed an (unrelated) issue with building the doc

@payno payno merged commit d27bb7a into silx-kit:main Jul 20, 2023
7 checks passed
@t20100 t20100 deleted the silx-view-slicing2 branch July 20, 2023 13:32
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.

3 participants