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

Climatology Test: Handing masked depths without zspan #107

Merged
merged 7 commits into from
Apr 2, 2024

Conversation

iwensu0313
Copy link
Contributor

@iwensu0313 iwensu0313 commented Apr 1, 2024

Summary

Copied changes from #104. Added a test to replicate missing values and no zspan set scenario #65 (comment). Proposed fix is to set MISSING flag at the end of check() so it doesn't get overridden by the GOOD flag.

Details

  • Added new test QartodClimatologyMissingTest to test_qartod.py to capture scenario described in Climatology test doesn't work with null values if zspan isn't provided #65. The scenario is when a user passes an array with a missing data point (which gets masked by ioos_qc) and does not set zspan (depth range). The result is that the missing value gets flagged w/ 1 (GOOD) instead of 9 (MISSING).
  • qartod.py - the change is moving where we set the MISSING flag to the bottom of the check() function so that it doesn't get overridden when we set the GOOD flags.

At the start of check() we currently set UNKNOWN and MISSING flags

# Start with everything as UNKNOWN (2)
flag_arr = np.ma.empty(inp.size, dtype='uint8')
flag_arr.fill(QartodFlags.UNKNOWN)

# If the value is masked set the flag to MISSING
flag_arr[inp.mask] = QartodFlags.MISSING

However, in this line near the end of the check() function, the slice keeps good data points AND all masked values. And assigns them the QartodFlags.GOOD flag which is a value of 1.

flag_arr[(values_idx & ~fail_idx & ~suspect_idx)] = QartodFlags.GOOD

We do not want MISSING flags (9) to be overridden with a GOOD flag (1), so assigning the MISSING flags at the end can prevent this.

@ocefpaf ocefpaf mentioned this pull request Apr 1, 2024
@iwensu0313 iwensu0313 marked this pull request as draft April 1, 2024 21:31
@iwensu0313
Copy link
Contributor Author

iwensu0313 commented Apr 1, 2024

Tests appear to have all passed, but pre-commit check failing. Need to investigate - converted to draft

@iwensu0313 iwensu0313 marked this pull request as ready for review April 1, 2024 22:43
@ocefpaf
Copy link
Member

ocefpaf commented Apr 2, 2024

Tests appear to have all passed, but pre-commit check failing. Need to investigate - converted to draft

Don't worry too much about pre-commits. Those are only code standards and sometimes we can fix those with:

pre-commit.ci autofix

@iwensu0313
Copy link
Contributor Author

Tests appear to have all passed, but pre-commit check failing. Need to investigate - converted to draft

Don't worry too much about pre-commits. Those are only code standards and sometimes we can fix those with:

pre-commit.ci autofix

Sounds good! I figured out how to fix this one. One of the pre-commit checks auto fixed the issue but the other one I had to fix manually in this commit 485d635. I definitely support sticking w/ code standards :)

@ocefpaf ocefpaf merged commit da1d6dd into ioos:main Apr 2, 2024
9 checks passed
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

Successfully merging this pull request may close these issues.

2 participants