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

Fix state-dependent callbacks #331

Merged
merged 1 commit into from
May 10, 2024
Merged

Fix state-dependent callbacks #331

merged 1 commit into from
May 10, 2024

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented May 7, 2024

Description

Some states require a callback function to determine the state transition value. That callback can depend on other state values. The existing code did not check that the state value was defined, in which case it would be None. In the case of #325 this would cause an exception. For the other state callbacks that are changed in this PR the existing code worked but somewhat accidentally.

I did an audit of all the callbacks. Callbacks with code like if state["eclipse_enable_spm"] were left unchanged since checking for None and returning would have the same effect.

Fixes #325

Interface impacts

None

Testing

Unit tests

  • Mac (new tests)
(ska3) ➜  kadi git:(fix-state-callbacks) git rev-parse HEAD                                                        
dd36d606367998375d5f97eb0efb1b4141bbf31e
(ska3) ➜  kadi git:(fix-state-callbacks) pytest
==================================================== test session starts =====================================================
platform darwin -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collecting ... Intel MKL WARNING: Support of Intel(R) Streaming SIMD Extensions 4.2 (Intel(R) SSE4.2) enabled only processors has been deprecated. Intel oneAPI Math Kernel Library 2025.0 will require Intel(R) Advanced Vector Extensions (Intel(R) AVX) instructions.
Intel MKL WARNING: Support of Intel(R) Streaming SIMD Extensions 4.2 (Intel(R) SSE4.2) enabled only processors has been deprecated. Intel oneAPI Math Kernel Library 2025.0 will require Intel(R) Advanced Vector Extensions (Intel(R) AVX) instructions.
collected 180 items                                                                                                          

kadi/commands/tests/test_commands.py ...............................................................................   [ 43%]
kadi/commands/tests/test_states.py .......................x.........................                                   [ 71%]
kadi/commands/tests/test_validate.py ....................                                                              [ 82%]
kadi/tests/test_events.py ..........                                                                                   [ 87%]
kadi/tests/test_occweb.py ......................                                                                       [100%]

Independent check of unit tests by Jean

  • Linux
ska3-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: anyio-4.3.0, timeout-2.2.0
collected 180 items                                                                                                    

kadi/commands/tests/test_commands.py ........................................................................... [ 41%]
....                                                                                                             [ 43%]
kadi/commands/tests/test_states.py .......................x.........................                             [ 71%]
kadi/commands/tests/test_validate.py ....................                                                        [ 82%]
kadi/tests/test_events.py ..........                                                                             [ 87%]
kadi/tests/test_occweb.py ......................                                                                 [100%]

=================================================== warnings summary ===================================================
kadi/kadi/commands/tests/test_commands.py::test_get_starcats_each_year[year0]
  /proj/sot/ska3/flight/lib/python3.11/site-packages/setuptools_scm/git.py:308: UserWarning: git archive did not support describe output
    warnings.warn("git archive did not support describe output")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================ 179 passed, 1 xfailed, 1 warning in 181.96s (0:03:01) =================================
ska3-jeanconn-fido> git rev-parse HEAD
dd36d606367998375d5f97eb0efb1b4141bbf31e

Functional tests

No functional testing.

@taldcroft taldcroft requested a review from jeanconn May 7, 2024 13:13
Comment on lines +1837 to +1840
# state["fids"] could be None at the beginning so wait until an AFLCRSET
# command to define it.
if state["fids"] is None:
return
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was not strictly necessary but makes this callback more consistent with others and makes the behavior more atomic.

@taldcroft taldcroft merged commit 978068e into master May 10, 2024
3 of 4 checks passed
@taldcroft taldcroft deleted the fix-state-callbacks branch May 10, 2024 10:27
@jeanconn jeanconn mentioned this pull request May 20, 2024
@javierggt javierggt mentioned this pull request Jun 26, 2024
javierggt pushed a commit that referenced this pull request Aug 20, 2024
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.

states.get_continuity fails
3 participants