Factor checks out of ACAReviewTable class

[taldcroft]

Description

This is to factor out the sparkles checks from being ACAReviewTable methods to independent check functions that return a list of messages. The motivation is to decouple these catalog checks from sparkles and make it easier to run them from starcheck or elsewhere.

Key updates:

• Move checks from being methods of the ACAReviewTable class into functions in a new checks.py module.
  • The functions take arbitrary arguments and return list[Message].
  • In the existing sparkles checks the first argument is always an ACACheckTable or subclass, but that is no longer a requirement.
  • Future checks should be written with more specific arguments where that would make sense.
• New Message dataclass and move MessagesList into new messages.py module.
• Factor out a new ACACheckTable class that provides the minimal functionality to perform checks. This separates out all the sparkles review functionality (making reports, roll angle optimization, etc) from the basic catalog checking functionality:
  • messages attribute to capture messages
  • Additional columns like row and col etc to facilitate checking the catalog
  • Some helper attributes.
• Update check tests to use both ACAReviewTable and ACACheckTable.

The refactoring part is going to be difficult for code review but the tests are solid.

Requires

• Add Row type aliases proseco#390 (for type annotations)
• masters channel in order to pass tests that depend on sun position.

Interface impacts

Testing

Unit tests

• Mac

(masters) ➜  sparkles git:(refactor-checks) git rev-parse HEAD
ee2f79e6e50c4f97c25163dbf9f132c591594db8
(masters) ➜  sparkles git:(refactor-checks) pytest            
===================================================== test session starts ======================================================
platform darwin -- Python 3.10.8, pytest-7.2.1, pluggy-1.0.0
rootdir: /Users/aldcroft/git, configfile: pytest.ini
plugins: timeout-2.1.0, anyio-3.6.2
collected 103 items                                                                                                            

sparkles/tests/test_checks.py ............................................................................               [ 73%]
sparkles/tests/test_find_er_catalog.py .....                                                                             [ 78%]
sparkles/tests/test_review.py ..................                                                                         [ 96%]
sparkles/tests/test_yoshi.py ....                                                                                        [100%]

===================================================== 103 passed in 34.39s =====================================================

Independent check of unit tests by Jean

• Linux

Functional tests

No functional testing.

[jeanconn]

It would make a little more sense to me to have check_catalog in checks, with an interface so that I could get the messages without writing back into the ACAReviewTable if desired. But I'm not passionate about it.

[taldcroft]

The idea was that if some other application wants to use the checks from checks.py, then that app has its own version of check_catalog that calls some or all of the checks.py checks. So the check_catalog() function in sparkles.core is specifically only for sparkles, but the checks in the checks module are somewhat public and modular.

Of course another application would be free to copy the current check_catalog with essentially no modification except the last few lines.