Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional Comments on Code Quality #23

Open
dostuffthatmatters opened this issue Mar 8, 2024 · 0 comments
Open

Optional Comments on Code Quality #23

dostuffthatmatters opened this issue Mar 8, 2024 · 0 comments

Comments

@dostuffthatmatters
Copy link
Contributor

dostuffthatmatters commented Mar 8, 2024

This issue is related to the review process for JOSS: openjournals/joss-reviews#6368

JOSS does not directly require some level of code quality - hence the following is independent of the review. Also code quality is often quite subjective. Nevertheless, the higher quality your codebase is, the easier it is to onboard new developers and the less likely they will develop their own tool because yours misses one feature.

  • Consider renaming WeatherQC._obtain_data to avoid confusing with the input_files._obtain_data function
  • The input_files._obtain_data function is almost 300 lines long. Consider breaking it into smaller parts which also forces you to structure the logic of your code
  • Remove the underscores from all the functions in input_files.py because they are used by other modules
  • Use a code formatter like Yapf of Black to format your code - removes visual clutter without you having to do it manually. VS Code has a "format on save" setting.
  • Consider return less tuples (like in calc_functions.calc_temperature_variables) and relying more in dicts where each item is a names value - i.e. return_value["monthly_k_not"] instead of return_value[3]. When possible, you might split up the functions so that each function calculates one thing.
  • The same is true for long lists of input variables (like in calc_functions.calc_humidity_variables). These make it so easy to accidentily pass tmin, tavg, tmax, ... instead of tmax, tmin, tavg.

I can give you a full review regarding code quality - if you want. But I also don't want to spam you with optional/stylistic comments ^^ Tools for automated code reviews can be very helpful too.

In general, I am a big fan of static typing for production codebases. With static type hints you can use Mypy to test your code on typing errors. However, I understand that it adds complexity to a codebase for developers who have not used it before. There are pro's and con's to it but I found it to save a lot of time because many bugs are caught by MyPy in (CI) tests instead of at runtime.

You can add Yapf/Black/Mypy options to a pyproject.toml file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant