melocelo / SC00039_Final_Project_MC

GNU General Public License v3.0
0 stars 0 forks source link

Changes and Improvements #1

Open alvaralmstedt opened 4 days ago

alvaralmstedt commented 4 days ago

Hi Melis!

You write a lof of stuff in the readme about what to install in the conda environment but you have actually created an environment.tml which is in the repository. I would just suggest the use use that to install the requirements like this:

conda env create --file=environment.yml

which would in theory install everything needed.

I had a problem though in where it seems like the nd2 package is not included, only nd2reader:

Traceback (most recent call last):
  File "/Users/alvaralmstedt/repos/course_students_HT2024/melis/SC00039_Final_Project_MC/Final_Assignment.py", line 21, in      <module>
    import nd2
ModuleNotFoundError: No module named 'nd2'

Additionally:

You never really explain that the .nd2 files need to be placed in the same directory as the python scrip. In general it is kind of bad practice to place data files inside the repository where the code reside. I suggest you use something like argv or argparse to take the path to the input as an argument to the script. That way the image files can be located anywhere.

And another nitpick: Perhaps you could delete the commented out # In[42] that I guess are artifacts from when you converted the code from an ipython notebook?

Le me know when you make some changes and ill check again. Not bad in general though :)

/Alvar

melocelo commented 4 hours ago

Hi Alvar,

Thank you for the feedback and suggestions. I see what you mean about the installation process. I have updated the instructions to focus on installing the environment as you suggested.

I have also added nd2 to the environment.yml file to make sure that it’s included, as it looks like only nd2reader was there by mistake since I was downloading by using pip in the first version and apparently I forgot to add it later. Thanks for letting me know.

And thanks for pointing out the commented-out code sections like # In[42]. I have cleaned those up to improve readability.

Regarding the .nd2 file placement, I'm actually using tkinter to allow the user to select the file interactively, so there's no requirement for the .nd2 files to be in the same directory as the script. This should allow users to select files from any location without additional command-line arguments. However, based on your suggestion I made a note of this in the README to avoid any confusion on that point even if it is not necessary.

If you think I have misunderstood any part of this or if there is anything else I should consider, please let me know.

Thank you for all,

Melis