Add qartod and config

[madrichardson]

No description provided.

[madrichardson]

I added qartod.py inside the esdglider folder, and I added qartod-config.yml in esdglider/data. Also, I messed up my branch name because I've been working with Quarto a lot, so the branch should've been qartod-module, but instead I accidentally named it quarto-module.

[smwoodman]

Please remove this file from the pr

[smwoodman]

I use xr.load_dataset(input_file) rather than these three lines

[madrichardson]

Ya that's a better approach, I'll update it.

[smwoodman]

Hi Madi - thanks for this! I've put some comments inline. A couple general ones, happy to discuss any of these:

For log messages, please use the syntax _log.info("QCing %s", var), rather than f-strings. I haven't run it in a while, but this comes from the pre-commit hooks that I (intermittently) use. I definitely need to pick a specific style guide, and have CoPilot go through and make this consistent :)

For me, having extra blank lines, eg between function calls makes the code less easily readable (example below). For esdglider, I'd like to stick to the principles of a) generally keeping code on one line is the line length is <80, and b) not having extra blank lines between lines in the same function.

# Please change 
final_flags = np.maximum.reduce(

    test_results

).astype("int8")

# to either:
final_flags = np.maximum.reduce(test_results).astype("int8")

# or:
final_flags = np.maximum.reduce(
    test_results
).astype("int8")

[madrichardson]

Hi Sam,

I'll make sure to fix all of these!

[madrichardson]

Hi Madi - thanks for this! I've put some comments inline. A couple general ones, happy to discuss any of these:

For log messages, please use the syntax _log.info("QCing %s", var), rather than f-strings. I haven't run it in a while, but this comes from the pre-commit hooks that I (intermittently) use. I definitely need to pick a specific style guide, and have CoPilot go through and make this consistent :)

For me, having extra blank lines, eg between function calls makes the code less easily readable (example below). For esdglider, I'd like to stick to the principles of a) generally keeping code on one line is the line length is <80, and b) not having extra blank lines between lines in the same function.

# Please change 
final_flags = np.maximum.reduce(

    test_results

).astype("int8")

# to either:
final_flags = np.maximum.reduce(test_results).astype("int8")

# or:
final_flags = np.maximum.reduce(
    test_results
).astype("int8")

Do you want me to remove the comments in the code (example below)?

EVALUATE ALL DATA VARIABLES

for var in ds.data_vars:
    # REQUIRE TIME DIMENSION
    if "time" not in ds[var].dims:
        continue
    # SKIP EXISTING QC VARIABLES
    if var.endswith("_qc"):
        continue
    # SKIP EXCLUDED VARIABLES
    #
    # Exclude metadata variables and redundant
    # coordinate variables that should not
    # receive QARTOD testing.
    if var in skip_variables:
        continue
    time_variables.append(var)
# LOG SUMMARY
_log.info(
    "Found %d time variables for QC",
    len(time_variables)
)
return time_variables

[smwoodman]

No, I think the comments are overall good, although I typically don't like them over logs because (unless it's really complicated) I think the log should be self-explanatory. I would format that block something like:

for var in ds.data_vars:
    # REQUIRE TIME DIMENSION
    if "time" not in ds[var].dims:
        continue

    # SKIP EXISTING QC VARIABLES AND EXCLUDED VARIABLES
    # {SMW note: I think fine to add more context here, if you feel it's necessary}
    if var.endswith("_qc") or var in skip_variables:
        continue

    time_variables.append(var)

_log.info(
    "Found %d time variables for QC",
    len(time_variables)
)

return time_variables

Thanks!

[madrichardson]

I updated everything based on your comments. If there's anything else, please let me know!

[smwoodman]

Thank you! This is looking good. I'll try out the fork tomorrow.

One other ask - could you add a notebook (to the 'notebooks' folder) that shows a representative example or two of using this functionality?

[madrichardson]

Yes! Sorry to get back late, I have been traveling. I'll make sure to add a notebook today.

[smwoodman]

Thanks, this is looking good. The outputs make sense

A couple more requests to complete the pr are below; sorry I didn't think to include these initially.

• Add 'ioos_qc' to the pyproject.toml and environment.yml files; it can go into the pyproject file as a normal dependency.
• Add yourself as an author in the pyproject.toml file
• Add a blurb to the CHANGLOG file. For instance, under the 'Module and function restructuring' heading: "Added a qartod module, for generating qartod flags using the ioos_qc package for the science dataset". Note: to make this change, you'll need to update your branch with the updates that I've pushed to esdglider:main. If that is a hassle, no worries, and I can add this line after merging in the PR.
• In the paths module, can you add a function (maybe get_path_qartod_config) for getting the path to the qartod-config.yml file? A la get_path_yaml. Sidebar, I need to rename the get_path_yaml function to differentiate it, probably to get_path_yaml_deployment_var. I'll do that in the main branch shortly.
• It would be quite useful to have a way of visualizing/summarizing the generated QC flags, that could be easy to run for every deployment. I'm thinking of timeseries plots of each QC variable, which could be added to the plots folder. Another options could be a numerical summary printed to the logs, eg of the number of suspect/fail flags per variable. But maybe you/Dale/Cara have a better idea for this?

[madrichardson]

I updated the requests you gave, but for visualizing/logging of the qc flags, what would be most helpful for you? Are you interested in seeing exactly when the bad flags occur or just generally how many flags (of each kind) there are?

[smwoodman]

Thanks @madrichardson! Updates look great. For visualizing/summarizing, I'm thinking both a histogram (or logged summary counts) of the number of each flag per variable, and then a timeseries of the flags. For the timeseries, either using color-coding to mark 'bad' flags, or have the plot only be of the 'bad' flags.

[madrichardson]

Hi Sam, the Glider DAC folks provided guidance for determining the thresholds for two of the tests for qc'ing. They already have functions for both: get_spike_thresholds() & get_rate_of_change_threshold(). Did you want me to incorporate these in the qartod module? I was thinking I would update qartod.py with these and then edit parts of the script so that if would load the qartod-config.yml, create the config-dictionary, compute the threshold statistics, replace the placeholder thresholds, and then update the config-dictionary so that the qartod-config.yml is never overwritten, just used as a template.

Also, I saw you were out of office until August, so no worries about getting back to me until then!

[smwoodman]

Thanks Madi. This all makes sense to me.

I like just updating the config dictionary, rather than having to update qartod-config.yml all the time. When adding functions, please just make sure to provide credit/links in the function docstring that points back to the DAC's original function

Also, maybe this happens by default, but will the output nc file have these function-calculated values as an attribute for the applicable quartod variable? Eg, will qartod_density_spike_flag have an attribute that contains the calculated spike thresholds? I think that could be useful for any future troubleshooting