Fix V&V processing error with Sqsh #303

jeanconn · 2024-10-03T15:33:26Z

Description

Fix V&V processing error with Sqsh.
Put the Sqsh calls in context managers instead of using an explicit conn.close that doesn't work on Sqsh.

Also change a couple of checks to use get() instead of checking "if self.info()["aspect_1_id"] is None" because for the nominal cases where there is a check on that value and the value doesn't exist, the key is also not defined and the error handling should be the same in that case.

Fixes issue that V&V processing was going through its paces but not getting the right information to update the status table.

Interface impacts

Testing

Unit tests

Linux

(ska3-mica) jeanconn-fido> pytest
======================================================== test session starts ========================================================
platform linux -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /proj/sot/ska/jeanproj/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 108 items                                                                                                                 

mica/archive/tests/test_aca_dark_cal.py ..................                                                                    [ 16%]
mica/archive/tests/test_aca_hdr3.py .                                                                                         [ 17%]
mica/archive/tests/test_aca_l0.py ...                                                                                         [ 20%]
mica/archive/tests/test_asp_l1.py .......                                                                                     [ 26%]
mica/archive/tests/test_cda.py ..............................................                                                 [ 69%]
mica/archive/tests/test_obspar.py .                                                                                           [ 70%]
mica/report/tests/test_write_report.py .                                                                                      [ 71%]
mica/starcheck/tests/test_catalog_fetches.py ...............                                                                  [ 85%]
mica/stats/tests/test_acq_stats.py ...                                                                                        [ 87%]
mica/stats/tests/test_guide_stats.py ....                                                                                     [ 91%]
mica/vv/tests/test_vv.py .........                                                                                            [100%]

================================================== 108 passed in 535.22s (0:08:55)

Independent check of unit tests by Javier

Linux:

jgonzale mica $ pytest mica
==================================================================== test session starts ====================================================================
platform linux -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /proj/sot/ska/jgonzalez/git
configfile: pytest.ini
plugins: anyio-4.3.0, timeout-2.2.0
collected 108 items                                                                                                                                         

mica/archive/tests/test_aca_dark_cal.py ..................                                                                                            [ 16%]
mica/archive/tests/test_aca_hdr3.py .                                                                                                                 [ 17%]
mica/archive/tests/test_aca_l0.py ...                                                                                                                 [ 20%]
mica/archive/tests/test_asp_l1.py .......                                                                                                             [ 26%]
mica/archive/tests/test_cda.py ..............................................                                                                         [ 69%]
mica/archive/tests/test_obspar.py .                                                                                                                   [ 70%]
mica/report/tests/test_write_report.py .                                                                                                              [ 71%]
mica/starcheck/tests/test_catalog_fetches.py ...............                                                                                          [ 85%]
mica/stats/tests/test_acq_stats.py ...                                                                                                                [ 87%]
mica/stats/tests/test_guide_stats.py ....                                                                                                             [ 91%]
mica/vv/tests/test_vv.py .........                                                                                                                    [100%]

===================================================================== warnings summary ======================================================================
mica/mica/archive/tests/test_cda.py: 13 warnings
mica/mica/report/tests/test_write_report.py: 94 warnings
mica/mica/stats/tests/test_acq_stats.py: 82 warnings
mica/mica/stats/tests/test_guide_stats.py: 18 warnings
mica/mica/vv/tests/test_vv.py: 2 warnings
  /proj/sot/ska3/flight/lib/python3.11/site-packages/tables/node.py:251: DeprecationWarning: `alltrue` is deprecated as of NumPy 1.25.0, and will be removed in NumPy 2.0. Please use `all` instead.
    self._v_objectid = self._g_open()

mica/mica/archive/tests/test_cda.py: 13 warnings
mica/mica/report/tests/test_write_report.py: 243 warnings
mica/mica/stats/tests/test_acq_stats.py: 9 warnings
  /proj/sot/ska3/flight/lib/python3.11/site-packages/tables/table.py:1513: DeprecationWarning: `sometrue` is deprecated as of NumPy 1.25.0, and will be removed in NumPy 2.0. Please use `any` instead.
    coords = [p.nrow for p in

mica/mica/report/tests/test_write_report.py::test_write_reports
mica/mica/report/tests/test_write_report.py::test_write_reports
  /proj/sot/ska/jgonzalez/git/mica/mica/report/report.py:203: DeprecationWarning: `alltrue` is deprecated as of NumPy 1.25.0, and will be removed in NumPy 2.0. Please use `all` instead.
    acqs = tbl.read_where("obsid == {}".format(obsid))

mica/mica/report/tests/test_write_report.py::test_write_reports
mica/mica/report/tests/test_write_report.py::test_write_reports
mica/mica/report/tests/test_write_report.py::test_write_reports
  /proj/sot/ska/jgonzalez/git/mica/mica/report/report.py:213: DeprecationWarning: `alltrue` is deprecated as of NumPy 1.25.0, and will be removed in NumPy 2.0. Please use `all` instead.
    guis = tbl.read_where("obsid == {}".format(obsid))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================================= 108 passed, 479 warnings in 351.86s (0:05:51) =======================================================
jgonzale mica $ git rev-parse HEAD 
3e2472e28bf50eec8a2512e291c4a099b1f6070c

Functional tests

While I prefer not to test "live", I did so here. Old code when unhandled does:

obsid dir /fido.real/conda/envs/ska3-flight-2024.1rc4/data/mica/archive/vv/01/01456_v06 already exists
moved VV files to /fido.real/conda/envs/ska3-flight-2024.1rc4/data/mica/archive/vv/01/01456_v06
removed directory /fido.real/conda/envs/ska3-flight-2024.1rc4/data/mica/archive/tempvv/tmpb4fq4trw
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[3], line 1
----> 1 mica.vv.process.process(1456, 6)

File /proj/sot/ska/jeanproj/git/mica/mica/vv/process.py:250, in process(obsid, version)
    248 tbl = h5.get_node("/", "vv")
    249 obi.set_tbl(tbl)
--> 250 obi.slots_to_table()
    251 tbl.flush()
    252 h5.flush()

File /proj/sot/ska/jeanproj/git/mica/mica/vv/core.py:304, in Obi.slots_to_table(self)
    302 def slots_to_table(self):
    303     save = self.info()
--> 304     if save["aspect_1_id"] is None:
    305         logger.warning("Table save not implemented for obsids without aspect_1_ids")
    306         return

KeyError: 'aspect_1_id'

New code updates correctly

obsid dir /fido.real/conda/envs/ska3-flight-2024.1rc4/data/mica/archive/vv/01/01456_v06 already exists
moved VV files to /fido.real/conda/envs/ska3-flight-2024.1rc4/data/mica/archive/vv/01/01456_v06
removed directory /fido.real/conda/envs/ska3-flight-2024.1rc4/data/mica/archive/tempvv/tmpug65mcrx
obsid 1456 is in table
updating obsid 1456 rev 6 in place

javierggt · 2024-10-24T17:53:53Z

mica/vv/core.py

@@ -269,7 +268,7 @@ def _save_info_json(self, file=None):
        logger.info("Saved JSON to {}".format(file))

    def slots_to_db(self):
-        if self.info()["aspect_1_id"] is None:
+        if self.info().get("aspect_1_id") is None:


Why are you using get instead of __getitem__? Is it that at some point you had this?

if self.info().get("aspect_1_id", None) is None

I thought using get() was the recommended dictionary public access to just get me the value and not throw an key error if the key is undefined and I thought the current code is equivalent to the line you've written for the "did you have this at some point" line. What would __getitem__ buy here?

Just making sure it was on purpose and not left from unrelated edit.

Ah - you can see it was on purpose from my commit history but it is a good point that I didn't call this out in the description!

javierggt

The change makes sense to me. I ran the tests on HEAD, but I could not run the functional tests, obviously.

javierggt · 2024-10-24T18:15:11Z

mica/vv/core.py

@@ -269,7 +268,7 @@ def _save_info_json(self, file=None):
        logger.info("Saved JSON to {}".format(file))

    def slots_to_db(self):
-        if self.info()["aspect_1_id"] is None:
+        if self.info().get("aspect_1_id") is None:


Just making sure it was on purpose and not left from unrelated edit.

jeanconn added 2 commits October 3, 2024 11:40

Use 'with' on Sqsh sqlsao calls instead of broken conn.close

29c0d07

Change some dict checks to gets

3e2472e

jeanconn force-pushed the vv-error branch from f5f70ce to 3e2472e Compare October 3, 2024 15:40

jeanconn marked this pull request as ready for review October 3, 2024 16:13

jeanconn requested review from javierggt and taldcroft October 3, 2024 16:14

jeanconn force-pushed the vv-error branch from ea27fea to 3e2472e Compare October 4, 2024 16:23

javierggt reviewed Oct 24, 2024

View reviewed changes

javierggt approved these changes Oct 24, 2024

View reviewed changes

Base automatically changed from no-aiprops-report to master October 24, 2024 18:45

jeanconn merged commit a66f099 into master Oct 24, 2024
4 checks passed

jeanconn deleted the vv-error branch October 24, 2024 18:46

javierggt mentioned this pull request Oct 30, 2024

Update sot/mica to 4.37.0 sot/skare3#1429

Closed

javierggt mentioned this pull request Nov 7, 2024

2024.11 sot/skare3#1427

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix V&V processing error with Sqsh #303

Fix V&V processing error with Sqsh #303

jeanconn commented Oct 3, 2024 •

edited

Loading

javierggt Oct 24, 2024

jeanconn Oct 24, 2024

javierggt Oct 24, 2024

jeanconn Oct 24, 2024

javierggt left a comment

javierggt Oct 24, 2024

Fix V&V processing error with Sqsh #303

Fix V&V processing error with Sqsh #303

Conversation

jeanconn commented Oct 3, 2024 • edited Loading

Description

Interface impacts

Testing

Unit tests

Functional tests

javierggt Oct 24, 2024

Choose a reason for hiding this comment

jeanconn Oct 24, 2024

Choose a reason for hiding this comment

javierggt Oct 24, 2024

Choose a reason for hiding this comment

jeanconn Oct 24, 2024

Choose a reason for hiding this comment

javierggt left a comment

Choose a reason for hiding this comment

javierggt Oct 24, 2024

Choose a reason for hiding this comment

jeanconn commented Oct 3, 2024 •

edited

Loading