Add task to pick ROIs
For #1
Merge request reports
Activity
added 2 commits
requested review from @payno
- src/ewoksixs/gui/roi_picker.py 0 → 100644
79 Scan.set_default_scan_path(scan_path) 80 81 above_list = [102, 106] 82 scan_above = Scan() 83 scan_above.load(above_list[0]) 84 for ii in above_list[1::]: 85 _tmp = Scan() 86 _tmp.load(ii) 87 scan_above._det_images += _tmp._det_images 88 img = scan_above._det_images.sum(axis=0) 89 numpy.save(img_path, img) 90 91 app = qt.QApplication([]) 92 window = RoiPicker(img) 93 window.show() 94 app.exec() - Comment on lines +69 to +94
two comments:
- having a main section in a file used as library is probably not a good idea. If this is to have an example then maybe an "example" module can be added. If this is to test then it can be moved to test. There is pytest fixture in ewoksorange for this. See https://gitlab.esrf.fr/workflow/ewoks/ewoksorange/-/blob/main/src/ewoksorange/tests/conftest.py?ref_type=heads
- Having test / examples that can only be run by esrf staff is probably not optimal. Can those datasets (including also the one used in test) be stored somewhere else (third part project, edna-site... ?)
changed this line in version 8 of the diff
I am not able to install it. I have the following error:
orangecontrib.ewoksixs.roi'. Traceback (most recent call last): File "/home/payno/dev/esrf/ewoks/venv/lib/python3.11/site-packages/orangecanvas/registry/discovery.py", line 309, in iter_widget_descriptions module = asmodule(name) ^^^^^^^^^^^^^^ File "/home/payno/dev/esrf/ewoks/venv/lib/python3.11/site-packages/orangecanvas/registry/discovery.py", line 552, in asmodule return __import__(module, fromlist=[""]) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/payno/dev/esrf/ewoks/venv/lib/python3.11/site-packages/orangecontrib/ewoksixs/roi.py", line 3, in <module> from ewoksixs.gui.roi_picker import RoiPicker File "/home/payno/dev/esrf/ewoks/venv/lib/python3.11/site-packages/ewoksixs/gui/roi_picker.py", line 11, in <module> from ixstools.scan import Scan, SpecPath ModuleNotFoundError: No module named 'ixstools' 2024-03-25 17:59:52,162:INFO:orangecanvas.main: Adding search path '/home/payno/dev/esrf/ewoks/venv/lib/python3.11/site-packages/orangecanvas/styles/orange' for prefix, 'canvas_icons'
and couldn't find out how to install
ixstools
(no pypi wheel to install it)It should be installed from the Git repo: https://gitlab.esrf.fr/ixstools/ixsscan
I'll add it to the requirements
added 2 commits
Thanks for the update.
- First one more suggestion regarding the setup.cfg. Maybe it could help to have a
full
section - this way we make sure usingpip install .[full]
we can access to everything. Something like:
full = PyQt5
Except this installation goes smoothly nice
-
I noticed that
ixstools
is an internal project. Does it make sense ? (simple question probably on beamline hand but if you want to have ewoksixs a bit open this will help) -
Then regarding the gui from a 'dummy user' perspective what I can be critic about is:
3.1 the output of
ROI picker
and the input ofcollect results along axes
have different names which is confusing3.2 maybe labels can be simplify. *
Named of the HDF5 file to save the ROI
-> output file ? HDF5 can be filtered from the FileDialog * scan numbers to sum for the ROI image:scan numbers
3.3 maybe understanding can be improve
-
Params of the SPEC path
: I have no clue what this can do and what are the valid / possible inputs - Regarding all the inputs I see sometime
bliss
and sometimespec
can it read from both ?
3.4 depending on the expected input of
Scan number of sum for the ROI Image
this can / should be improved.- once we know the possible valid scan numbers (if we know them) maybe it a dedicated interface can be proposed to select / unselect the scan number.
- on the collect results along axes (not part of the PR) I also noticed the following:
- there is a lot of input which are not making much sense. Maybe some tooltips can help there.
- I see that there is
Axis names
andAxis_choices
. Why is there one with underscore and not the other ? Same forpath_to_spec
andspec_file_name
. - For now I also see that there is some
edf
(spec) and hdf5 melt. What is the reason ? For now you expect all dataset to be stored unsed .edf and you save the processing to .hdf5 ?
-
I cannot access
/data/id21/inhouse/wout/ixstools/test_data
so not able to process more test. -
icons could also help the users to understanding the meaning of each widget and find them more quickly.
Edited by payno- First one more suggestion regarding the setup.cfg. Maybe it could help to have a
- Good idea
(edfb833a) - I'll have to check with Christoph
3.1. Good point, I'll rename those (c17cb149)
3.2
(in next PR) 3.3 My next PR will change
Params of the SPEC path
into more meaningful controls. For now, it only accepts SPEC but in the future (summer), it should accept BLISS paths.3.4. Perhaps. I'll discuss this specifically with Christoph
-
Again, I hope that by discussing with Christoph, I'll be able to improve the controls. But you are right, there are a few typos to fix. As for the
edf
vshdf5
, I am merely following whatixstools
is doing -
You don't have a mounting of
/data
on your computer ? -
Good idea
(in a future PR)
Edited by Loic Huder- Good idea
mentioned in commit bfff0d3f
mentioned in merge request !5 (merged)