Current Codebase Review

[lewismcnish]

Overview

All new members of the team to perform a comprehensive review of the current state of the software and identify any issues, bugs, quality of life improvements and potential feature enhancement/implementation capabilities. Add any comments directly onto GitHub and ping relevant team members to view these comments via teams/whatsapp/email If no changes can be made add a quick comment specifying this

Definition of Done

1. A base level understanding of the codebase and what each of the scripts do.
2. Relevant comments/no comments on each section of code
3. A final comment under this issue that outlines the key comments (brief list)

Resources

Check the Teams software chat for link to relevant documentation Have a look through the folder linked, includes general information and software data for use in algorithms

[JazzyMaxine]

1. Is calling shell commands from the notebooks good practice? It seems dangerous for something as trivial as pip installs. One could accidentally pollute their environment if they didn't enter a venv before running the notebook, whereas if these shell commands weren't there then they would just get a "module not found" error and realise their mistake. I'd recommend including a requirements.txt file instead.
2. "Aberdeenshire.tif" doesn't seem to exist.
3. Index error in clouds notebook, trivial to fix.

The actual code seems good, but organisation and protocol are lacking.

I will catch up in the meeting tomorrow and figure out how to proceed.

[lewismcnish]

Yes this habit of shell commands in python notebooks is not good, and should just be done in venv, requirements.txt will be an issue, I Don't have wifi at home at the moment so struggling to get a lot done on this side will do more tonight Aberdeenshire.tif is in the teams channel, this needs included in Github LFS Not sure about index error, not looked at this this week

[2800957K]

1) "Aberdeenshire.tif" does not exist. The file has been hardcoded. Why not use it as a parameter? 2) The line self._reset_state in init is incorrect. It should be self._reset_state(), as it’s a method call. 3) Index error in cloud mask notebook 4) Index error in Seg_models_test_for_Coast? (not sure if its happening just in my laptop but worth a check) 5) Index error in Seg_models_test_for_Coast_with_processing? (not sure if its happening just in my laptop but worth a check).

[lewismcnish]

@2800957K Yep aware of this with the .tif file, needs resolved if it hasn't been already,

the index error in both seg notebooks is a varying issue, I have it only after having run the code once already, don't know if its a .ipynb artefact or bad code but if an issue for bug fix could be created for this that would be great.

self._reset_state can be fixed with issue, this is probably just an oversight (unfamiliarity with OOP)

Index error in cloud mask im unaware of, probably same cause as above, worth creating an issue and fixing

Thanks for the comments

[Scriptminer]

Apologies for the delay, here were the issues I noticed (all stylistic, apart from the first one):

In cloud-mask/cloud-mask.ipynb

• In compute_brightness, should division be by 4, not 3, since there are 4 channels?
• In create_cloud_mask, these magic numbers should be extracted and put in a config file or similar.

`unet/Set_models_test_for_Coast_with_processing.ipynb

• Would be better to avoid use of shell commands for file fetching and processing.

General

• Docstrings for functions would be good, ideally with a standard docstring format for functions and classes.