search

aodn / imos-toolbox

Graphical tool for QC'ing and NetCDF'ing oceanographic datasets
GNU General Public License v3.0
46 stars 31 forks source link

IMOS utilities and OceanContour Parser, closes #691 #689

Closed ocehugo closed 3 years ago

ocehugo commented 3 years ago

This is an aggregated of two dependent branches - IMOS Utilities and the Ocean Contour Parser. Although it's big, there quite a bit of dog fooding in there for the utilities and the parser, so better to review together.

The branch here implements further Utilities that I plan to use in any new parser. There is quite some tooling here that helps to reduce code size, hopefully create cleaner code, and even reduce the number of source files (e.g. test files) since now docstrings may work as simple tests. Most of the utilities are around creating variables/dimensions/etc, data access patterns, and assignment of data to toolbox structures. Some very small bugfixes are here too since they were detected some usage patterns.

The OceanContour parser is implemented as a class and uses some of these functions.

@lbesnard - Make sure you start the review after merging https://github.com/aodn/imos-toolbox/pull/687 and do a relink of this PR to master (on the edit button of the PR title). This should update this PR to the recently merged master changes and remove the already reviewed commits.

ocehugo commented 3 years ago

@lbesnard - just did sync.

ocehugo commented 3 years ago

I think it would be good to stick to snake case for function naming.

Did you mean camelCase right? I'll be honest, the camelCase is not my cup of tea, but I'm trying to stick to it, particularly for functions that can be access from the prompt.

The camelStyle is quite a problem for me because I'm too used to disambiguate types when reading the code (a-la python). Implicit rules I adopted:

  1. keep camelCase for any generic top level function/class. Exception is the assertion functions like isstruct, because this follows core matlab functions.
  2. keep snake_case for internal methods or package methods. This is the case in +IMOS and class methods.
  3. keep uppercase chars for globals or globals in the top-level scope. I don't think I used this rule yet, no need so far.

The scope of this PR is well above creating a simple parser.

Yes, it's plumbing first, parser next. The parser would be way more complicated without the plumbing, plus the plumbing allows me to write better code and do deep refactorings if needed.

ocehugo commented 3 years ago

@lbesnard - see new commits.