SpreadsheetAbsolutesFactory - absolute observations need valid flags from summary spreadsheet

• modify SpreadsheetSummaryFactory.py to set the valid flag in each Absolute object, and return the full reading regardless of whether any invalid absolutes existed;
• modify SpreadsheetAbsolutesFactory.py to use the updated SpreadsheetSummaryFactory to retrieve valid flags from summary spreadsheet files, and add these to the list of absolutes returned in each reading.

Details
Residual absolute observations were calculated and recorded in special spreadsheets for over 10 years. These spreadsheets have no quality flag to speak of, but they do hold the individual residual observation measurements, and are therefore critical to transferring previous residual measurements to the new operations database.

Reviews of these absolute observations were recorded in a separate "summary" spreadsheet, including a "rejected" flag in the J column corresponding to each absolute/baseline pair. These summary spreadsheets do not contain individual residual observation measurements, and are therefore not a complete record of absolutes needed for the new operations database.

There are currently two different factories in geomag-algorithms: SpreadsheetAbsolutesFactory and SpreadsheetSummaryFactory. The first is already used in the copy_absolutes.py application module in geomag-algorithms' processing folder. The second should be incorporated into the first, probably in the _parse_absolutes() definition, and used to determine if a given absolute measurement is "valid" or not.

One problem is that SpreadsheetSummaryFactory does not currently set and return the Absolute.valid object, but rather check to see if all absolutes for a particular reading are valid before returning it. This is not desirable, and should be modified to set the valid flag, and return the full reading regardless of whether any valid flags are true or not.

assigned to @awernle

While testing SpreadsheetSummaryFactory.py, I noticed some errors that may need to be handled in either this issue or a new one. At least the _parse_readings function does not handle NoneType properly.

For example, if we are pulling from a cell in the spreadsheet that is empty, a NoneType is returned. This causes mathematical operations to fail and we get an error like "Exception has occurred: TypeError unsupported operand type(s) for /: 'NoneType' and 'int'". Specifically, these errors show up for this code:

        Absolute(
                    element="D",
                    absolute=Angle.from_dms(
                        degrees=sheet[f"C{d_n}"].value, minutes=sheet[f"D{d_n}"].value
                    ),
                    baseline=sheet[f"H{d_n}"].value / 60,
                    starttime=parse_relative_time(
                        base_date, "{0:04d}".format(sheet[f"B{v_n}"].value)
                    ),

I think we need to add some exception handling, otherwise the entire script crashes.

(edited to format snippet for readability -EJR)

Agreed. I recommend adding exception handling to this issue/MR. Please do so as separate commit(s).

Out of curiosity though, what did the summary spreadsheet look like that triggered this problem? Did it have fewer than 4 sets? Was it missing one of H, D, or Z for a given set?

Yep, the summary spreadsheet only had two sets instead of the standard four. I am sure there are many other cases like this though.

Will do!

I added some exception handling to the script in multiple places but I also added a dynamic range to the _parse_readings function so that we are not always assuming 4 sets.

Both of these changes together might not be necessary because we should always be looping through the correct number of sets so feel free to cherry-pick changes. The exception handling might be nice to have regardless but if we were to keep the exception handling and get rid of the dynamic range, then we would always return 4 sets of absolutes even if some of them were only None values. This might be important to consider.

Let me know if I can clarify anything.

We discussed in real-time, but for posterity: I think it's fine for there to be some "redundancy" here. More accurately, I see the exception handling as just good coding form, even if you modified other parts of the code to avoid the exceptions. I wouldn't change anything described above.

marked the checklist item modify SpreadsheetSummaryFactory.py to set the valid flag in each Absolute object, and return the full reading regardless of whether any invalid absolutes existed; as completed

I think another important thing to note is that I have defined the valid flag for each Absolute object to be either "True" or "False" and if there are any false absolutes this leads to the Reading object having a valid flag of "None". If all absolutes are true, then the Reading object has a valid flag of "True".

This does not particularly bother me but let me know if you want me to write some conditional to define the valid flag of the Reading object depending on the valid flag of the Absolute objects.

I have edited the SpreadsheetAbsolutesFactory to call the SpreadsheetSummaryFactory and match the valid flags based on time. I ended up choosing time because I did not trust any other variables.

During our Thursday meeting I would like to talk about the Summary Spreadsheets and how reliable they are because I have noticed some potential errors. I am worried about matching the valid flags based only on the correct times if the numbers entered into the Summary Spreadsheets are wrong. This makes me wonder if we should have a secondary check to make sure the numbers match up between sheets, however "correct" numbers seem to have varying sig figs in some cases but this may be intentional.

We discussed in real-time, but for posterity: after discussion and consideration, we decided that it is not the purpose of this task to correct for user transcription errors in the Summary spreadsheets. The expectation is that these transcription errors are very rare. At worst, we mis-label actual measurements as valid/invalid, but none of these are ever likely to be re-processed, so there is little reason to fix them. In short, don't implement a secondary check for matching observations between measurement and summary spreadsheets; time stamps are good enough.

marked the checklist item modify SpreadsheetAbsolutesFactory.py to use the updated SpreadsheetSummaryFactory to retrieve valid flags from summary spreadsheet files, and add these to the list of absolutes returned in each reading. as completed

Please close this issue when !213 (merged) gets merged.

closed