small changes for consistency

[GL-S]

Proposed changes

These are some small changes to improve the consistency of the functions used to plot the land cover static images and the animations. A more detailed explanation of the variable that defines the output width was also included.

Checklist

If this is a notebook, then have you:

• Checked the structure of the notebook follows our DEA-notebooks template
• Removed any unused Python packages from Load packages
• Removed any unused/empty code cells
• Removed any guidance cells (e.g. General advice)
• Ensured that all code cells follow the PEP8 standard for code. The jupyterlab_code_formatter tool can be used to format code cells to a consistent style: select each code cell, then click Edit and then one of the Apply X Formatter options (YAPF or Black are recommended).
• Included relevant tags in the final notebook cell (refer to the DEA Tags Index, and re-use tags if possible)
• Tested notebook on the DEA Sandbox
• Cleared all outputs, run notebook from start to finish, and save the notebook in the state where all cells have been sequentially evaluated
• If applicable, update the Notebook currently compatible with line below the notebook title to reflect the environments the notebook is compatible with
• Check for any spelling mistakes using the DEA Sandbox's built-in spellchecker (double click on markdown cells then right-click on pink highlighted words). For example:

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[robbibt]

Hey @GL-S, this is completely optional, but we do already use width_pixels in other DEA Tools functions, e.g. here: https://github.com/GeoscienceAustralia/dea-notebooks/blob/develop/Tools/dea_tools/plotting.py#L384-L388

If we were going to standardise, it might be better to go the other way around, e.g. use width_pixels everywhere instead. But it's a minor thing! 🙂

[GL-S]

Thanks @robbibt !
You are right, the best way to standardise it would be to use width_pixels, as in xr_animation. In the end, I opted for keeping out_width because I noticed that width_pixels was used slightly differently from xr_animation, even though the name was the same. In practice, the functions in landcover.py use that input variable as a width in inches, rather than in pixels. I decided to simply change the name without modifying what the code does, to save time in testing I didn't break anything and in changing the notebooks.

We had some ideas for further improvements to the code that we haven't had time to implement yet. I think it would be a good idea to add this consistency issue to the list. Maybe we could work on these changes during the next innovation day (if there isn't any more urgent task), when we have some time allocated specifically for this?

[robbibt]

Looks great - let's wait for the tests to complete, then feel free to squash-and-merge

We had some ideas for further improvements to the code that we haven't had time to implement yet. I think it would be a good idea to add this consistency issue to the list. Maybe we could work on these changes during the next innovation day (if there isn't any more urgent task), when we have some time allocated specifically for this?

Yeah that sounds like a great idea!

[robbibt]

FYI @GL-S, you've had to wait for the entire slow test suite to run on your recent PRs because of the changes to DEA Tools python funcs - usually it will only test the modified notebooks test and be a lot faster 🙂

[GL-S]

Thanks for the explanation @robbibt! Interesting to know.
I didn't have much experience with PRs that include only notebooks, so I didn't realise these were slower... I just assumed it was the standard wait time!

[robbibt]

This entire repo integration test is actually pretty brand new (as of last month) - I'm still getting used to the wait time myself, but this way we can be 100% sure changes don't break anything else in the repo... which is pretty handy!