search

epoc-ed / GUI

Graphical User Inteface for JungFrau integrated in JEOL TEM
0 stars 0 forks source link

output dir as read-only [feature/redis_config] #54

Open tgruene opened 1 month ago

tgruene commented 1 month ago

file_ops_diff_TG.txt I took a quick look at feature/redis_coonfig. It looks neat. I would like to suggest two changes, as in the attached 'git diff' file: 1) chage 'User name' into 'PI name' 2) make self.base_directory_input.setReadOnly(True) (and consequently remove options to modify this directory, like the directory browser. I didn't want to make this a pull request, for I wasn't sure of any further consequences I might have overlooked. I just checked it quickly. The reason for 2): users should not be able to modify the base output dir, it should be in the hands of the instrument / data responsible. Only downstream modifications would be ok for me.

tgruene commented 1 month ago

Hi Khalil, I got a comment by email, which I don't see here on the web interface ... Anyway. I would like only the part '/data/jungfrau/psi' (and later '/data/jungfrau/jem2100plus') as non-modifiable by the operating user. It should only be modified by an admin, but not from the GUI, only from a separate redis DB client. See the diff file, that I included - it should not require any other modification.

kferjaoui commented 1 month ago

Hi Khalil, I got a comment by email, which I don't see here on the web interface ... Anyway. I would like only the part '/data/jungfrau/psi' (and later '/data/jungfrau/jem2100plus') as non-modifiable by the operating user. It should only be modified by an admin, but not from the GUI, only from a separate redis DB client. See the diff file, that I included - it should not require any other modification.

Hi Tim, Yes i deleted the reply as it didn't make much sense :) it would be reckless to offer the user (not admin) the possibility to change the folder where saved h5 files would be as they may inadvertently change the 'root directory' portion in it and then have mismatch between root path and dataset path...

Changes have been made as suggested.

I am also thinking of modifying the tiff path so that it includes two parts: (1) first line edit (read-only) where the dataset path would display (2) second line edit (modifiable) for the name of the tiff file

Basically, I do not want the user to be stuck with the name 'file' or have to navigate the tree, find the file and then manually rename it...

Let me know if this makes sense.

tgruene commented 1 month ago

Looks good to me for the screenshots. Why do you save TIFF, rather than e.g. PNG?

kferjaoui commented 1 month ago

No reason...Erik coded a tiff writer from the old gui so i kept it but we can change it to save captures in other formats if you'd like.

Btw, one feature of pyqtgraph, is that you may save a picture of the plot you are viewing by right-click on the frame and then choosing 'export'. It opens a menu where you can choose the format of your choice (png, jpg, tiff...) [see attached capture]

export_png_jpg

erikfrojdh commented 1 month ago

Yes tiff was only chosen for historical reasons. I think we should switch to png as default.

This seems to be a good library to use:

Using the save function allows us to save the actual pixel values for analysis instead of the displayed image with colormap. Additionally one might want to accumulate more frames than what is used in the live view.

erikfrojdh commented 1 month ago

Follow up: now I know why I decided not to use PNG

In grayscale mode it only supports up to 16bits per pixel. This is ok for saving a screenshot, even one with the colormap applied but will result in loss of information. TIFF on the other hand is more flexible and we can write almost any data type.

Should we keep tiff or move to hdf5? Or better png since the images are not going to be analyzed?

tgruene commented 1 month ago

Personally, I do not use the screenshot utility at all. I record images with the writer in HDF5-format, and convert later. TIFF seems plausible with the >16-bit depth. From my point of view, we can thus keep it in TIFF, as it is now. TIFF is really only a container, rather than an image format, isn't it?

erikfrojdh commented 1 month ago

If you don't use it... maybe we should remove it?

It was added as a way to write converted images with the old receiver. Now since we always convert I'm not sure we need it. Maybe better with an option to save the next N frames as hdf5

tgruene commented 1 month ago

It is my personal habit to record images with the writer. Personally, I also consider images of the particles as data, therefore I record them with the writer. Often I pan around, especially if the crystal is larger than the field of view. Sometimes I zoomed in while recording data, in order to have high resolution images. I suggest we collect opinions during the next EPOC meeting.