port change from #35

[ocefpaf]

This PR ports the only missing change from #35. The problem is that I'm not familiar with the expected results to fix the failing tests. With this change this test case passes but I cannot confirm the original poster test case.

[ocefpaf]

My guess is that this one should "fail" (marked as 3, right?) b/c the z-values are outside of the range. @eldobbins I'm not sure you are still working on this but it seems that you were the one looking into this problem in #65.

[iwensu0313]

Hi @ocefpaf, I would think we want that last result to be a 2 for "QARTOD not applied" since the data does not include information about depths so we can not apply QARTOD, instead of 3 for "QARTOD applied and this data point is a bad data point and failed the test".

My interpretation of QARTOD manuals is that 3 should be reserved for data that has been identified as bad. But in the context of the climatology test, this is more just that we are unable to use the climatology test on this dataset.

(Note: Liz is no longer with Axiom)

[iwensu0313]

https://cdn.ioos.noaa.gov/media/2020/07/QARTOD-Data-Flags-Manual_version1.2final.pdf

[ocefpaf]

Got it. I guess we need to look again into #35. I'll mark this one as a draft for now. Feel free to edit/resend it if you see a clear path forward.

[iwensu0313]

Ah, okay -

TLDR: So a flag of 3 does make sense (good w/ the change you made), bc we did do a check on the value 79 against the only applicable vspan ((40,50)). A configured vspan is applicable if data point falls in the tspan (time range) and zspan (depth range, where provided). If zspan is not provided, it will ignore the depth value and only match against the tspan.

Details (and so I don't have to re-remember in the future; feel free to ignore):
I spent some more time in the ioos_qc.qartod and the test_qartod unit tests this morning in debug mode to better understand how the test configurations are working (prior to Axiom, my interaction with ioos_qc was more of a user, applying it to ocean data and had never looked into the codebase before to see how it actually works behind the curtains).

(Looking at the screenshot above that I posted, 3 is actually Suspect not Fail! Not sure how I missed that last week)

Regarding the last test data point:

            # Not run, has depth that's outside of given depth ranges
            (
                np.datetime64('2012-01-02'),
                79,
                101
            )

The only climatology config that gets applied is this one (line 735):

        self.cc.add(
            tspan=(np.datetime64('2012-01'), np.datetime64('2013-01')),
            vspan=(40, 50),
        )

In this unit test, the code basically first checks to see if the test data falls within the time and depth range provided (of each of the 5 configs in setUp lines 725-750) to determine if the config should be applied to this test data. If it falls within specified time and depth ranges, then it will run the test for the value (in this case 79) against the vspan ((40,50)) to see if it is within/outside the range. And since we don't set fspan in any of the 5 configs in this unit test, if a data point falls outside of vspan, it will get a 3. It can only get a 4 if we set fspan.

If the depth associated with data point we are testing (e.g. 101) is outside of any given depth ranges (e.g. (0,10), (10,100)), the data point (e.g. 79) will still be tested if there is a config w/ no zspan specified AND the time associated with the data point (e.g. np.datetime64('2012-01-02')) falls within the tspan. The climatology test does not and should not flag a data point as suspect or fail if the depth falls outside of the given depth range. Only if the value falls outside of the relevant vspan. No zspan specified means it will be applied to a data point regardless of the depth IF the time associated with that data point falls within tspan.

[iwensu0313]

I just approved. But double checking that the changes in this PR work with the example cases provided here #65

[iwensu0313]

I just approved. But double checking that the changes in this PR work with the example cases provided here #65

Okay, we have to look into one more thing (but I need to pause and come back to this). I tested the test code snippet provided by lgarzio in #65 with the changes in this PR and it's still not producing the expected result (it does work when you include z_span). code snippet:

from ioos_qc.qartod import ClimatologyConfig, climatology_test
c = ClimatologyConfig()
c.add(
  tspan=['2021-06-01', '2021-09-01'], 
  vspan=[3.4, 5]
  )

inp = np.array([0.     ,     np.nan, 0.     ,     np.nan,     np.nan,     np.nan,     4.16743, 4.23101,     4.23322])

t = np.array(['2021-07-16T19:01:01.313999872', '2021-07-16T19:01:02.315000064',
       '2021-07-16T19:01:02.903000064', '2021-07-16T19:01:03.903000064',
       '2021-07-16T19:01:04.903000064', '2021-07-16T19:01:05.903000064',
       '2021-07-16T19:01:06.903000064', '2021-07-16T19:01:07.336000000',
       '2021-07-16T19:01:07.903000064'], dtype='datetime64[ns]')

z = np.array([0.     ,     np.nan, 0.     ,     np.nan,     np.nan,     np.nan,     0.08931513, 0.15878244,     0.11908684])

result = climatology_test(config=c,
                        inp=inp,
                        tinp=t,
                        zinp=z)

we should get this result (bc 4 data points are np.nan):

masked_array(data=[3, 9, 3, 9, 9, 9, 1, 1, 1],
             mask=False,
       fill_value=999999,
            dtype=uint8)

but I get

masked_array(data=[3, 1, 3, 1, 1, 1, 1, 1, 1],
             mask=False,
       fill_value=999999,
            dtype=uint8)

[iwensu0313]

Alright, I have a couple suggested changes:

• Adding a test to capture the missing data scenario described above, first mentioned in Climatology test doesn't work with null values if zspan isn't provided #65 (it would pass the missing data scenario to self._run_test and ensure that the flag is 9 not 1)
• Moving flag_arr[inp.mask] = QartodFlags.MISSING from the top to the bottom of check() in qartod.py, which will fix it, but there's one more behavior that is kinda funky (can explain in a follow on comment)

I tried to push my local changes for the above two things to your branch here, but I got this message bc I don't have write permissions for ioos_qc

How should I proceed?

[ocefpaf]

I tried to push my local changes for the above two things to your branch here, but I got this message bc I don't have write permissions for ioos_qc

I don't have enough permissions to add you but I'll request folks who do to add you. Meanwhile you can:

1. Send edits to my fork's branch (IMO that is too complicated if you don't want to mess with git);
2. Just send a fresh PR copying my changes. I don't care about authorship of the commits but I do care about getting you (and others) up to speed with ioos_qc.

IMO 2 is easier. Let me know what you want to do.

[ocefpaf]

Closing this one in favor of #107.