search

BohndiekLab / patato

PATATO: A Python PhotoAcoustic Tomography Analysis Toolkit
MIT License
9 stars 4 forks source link

Minor comments #34

Closed MengjieSHI closed 9 months ago

MengjieSHI commented 9 months ago

Hi @tomelse, I am running the examples, and they work perfectly:) Here are some minor comments I have for further improvements.

  1. I guess the folder named study contains the folder named Scan_1, Scan_2. It would be great to rephrase the explanations a bit, as the files were also named Scan_1.

patato-import-ithera /path/to/ithera/study/folder /path/to/data/folder

  1. For the GUI of tuning the speed-of-sound, a. The captions of the subfigures were missing. Did the right figure show the profile along x or z direction? b. Is it possible to reopen the GUI using the same command after I saved the speed-of-sound and quit it? I failed on this, but I am wondering if there is a need to modify the speed-of-sound afterwards.

  2. I am not very clear about how to rename the ROIs on the GUI. I clicked the rename button, but it seems it didn't work as expected.

  3. A link is missing at line 3, first para, Examples 4. Figures got overlapping in the results of Block [10].

Thanks!

This is part of JOSS review

tomelse commented 9 months ago

Glad to hear the examples run perfectly! And apologies for the delayed response.

Could you elaborate a bit what you mean for part 1? Do you mean clarify in the Quick start guide in the documentation?

I've updated both the GUIs for ROI drawing and speed of sound tuning. I've wanted to update them for a long time... I've removed the line plot functionality as I don't think it's that helpful. Feel free to try it out and let me know if there's anything you think I've missed.

The same commands can still be used, but with no arguments this time.

patato-set-speed-of-sound
patato-draw-roi

I will work on number 4 today.

Thanks very much!

tomelse commented 9 months ago

@MengjieSHI I've now fixed all of the issues you mentioned (see here: https://patato.readthedocs.io/en/latest/examples/04_timeseriesanalysis.html).

MengjieSHI commented 9 months ago

Hi @tomelse Thanks for your updates! Yes, I mean it would be better to clarify a bit for Part 1 as the folder and files have the same name in your example.

I downloaded the latest release 0.5.12 from GitHub but seems the updates on GUIs were not applied, either for the commands. It is quite wired, anything you think I could try? Thanks!

tomelse commented 9 months ago

Have you run

pip install --upgrade patato

or pip install --upgrade -e .

to install the updated version?

I'll take a look at the documentation tomorrow, thanks

MengjieSHI commented 9 months ago

Yes, just restarted my computer and everything works well now:)

The new GUIs look really great!

It seems there is no return after I clicked save speed of sound? I also got some warnings after closing the GUI. image