Skip to content

Merge each OBS command from an obsid-changing CMD_EVT into the prior …#382

Merged
taldcroft merged 5 commits intomasterfrom
merge-obsid-obs-redux
Mar 25, 2026
Merged

Merge each OBS command from an obsid-changing CMD_EVT into the prior …#382
taldcroft merged 5 commits intomasterfrom
merge-obsid-obs-redux

Conversation

@taldcroft
Copy link
Copy Markdown
Member

@taldcroft taldcroft commented Mar 10, 2026

Description

The processing to generate kadi observations (OBS commands) splits an observation when the obsid changes. This allows capturing undercover observations etc. However, manually commanded obsid changes following an SCS-107 or an ECS measurement are somewhat different because their timing is unrelated to normal observation flow (maneuver => Kalman dwell).

In using the new scheduled obsid capability in practice, it became apparent that splitting an observation for a manual obsid change was undesirable. A query for a particular scheduled obsid is expected to return the single entire observation, not the two splits.

This PR does that merging within get_observations (and therefore get_starcats as well). The PR code provides another complementary description of what is going on here that might be helpful.

In case this behavior ends up being problematic somewhere, the PR also adds a configuration option to disable the merging.

Interface impacts

Testing

Unit tests

  • Mac
(ska3) ➜  kadi git:(merge-obsid-obs-redux) git rev-parse --short HEAD
c874604
(ska3) ➜  kadi git:(merge-obsid-obs-redux) pytest
======================================= test session starts ========================================
platform darwin -- Python 3.13.11, pytest-9.0.2, pluggy-1.6.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: anyio-4.12.1, timeout-2.4.0
collected 310 items                                                                                

kadi/commands/tests/test_commands.py ....................................................... [ 17%]
.....s..............................                                                         [ 29%]
kadi/commands/tests/test_commands_v2.py ssssssssssssssssssssssssssssssssssssssssssssssssssss [ 46%]
ssssssssssssssssssssssssssssssssssss                                                         [ 57%]
kadi/commands/tests/test_filter_events.py ..                                                 [ 58%]
kadi/commands/tests/test_states.py ...............................................x......... [ 76%]
.................                                                                            [ 82%]
kadi/commands/tests/test_validate.py ......................                                  [ 89%]
kadi/tests/test_events.py ..........                                                         [ 92%]
kadi/tests/test_occweb.py .......................                                            [100%]

====================== 220 passed, 89 skipped, 1 xfailed in 113.23s (0:01:53) ======================
(ska3) ➜  kadi git:(merge-obsid-obs-redux) env KADI_CMDS_VERSION=2 pytest
======================================= test session starts ========================================
platform darwin -- Python 3.13.11, pytest-9.0.2, pluggy-1.6.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: anyio-4.12.1, timeout-2.4.0
collected 310 items                                                                                

kadi/commands/tests/test_commands.py sssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 17%]
ssssssssssssssssssssssssssssssssssss                                                         [ 29%]
kadi/commands/tests/test_commands_v2.py .................................................... [ 46%]
....................................                                                         [ 57%]
kadi/commands/tests/test_filter_events.py ..                                                 [ 58%]
kadi/commands/tests/test_states.py ...............................................x......... [ 76%]
.................                                                                            [ 82%]
kadi/commands/tests/test_validate.py ......................                                  [ 89%]
kadi/tests/test_events.py ..........                                                         [ 92%]
kadi/tests/test_occweb.py .......................                                            [100%]

====================== 218 passed, 91 skipped, 1 xfailed in 116.28s (0:01:56) ======================

Independent check of unit tests by Jean

  • OSX
(ska3-latest) flame:kadi jean$ pytest
======================================================================== test session starts ========================================================================
platform darwin -- Python 3.13.11, pytest-9.0.2, pluggy-1.6.0
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: anyio-4.12.1, timeout-2.4.0
collected 314 items                                                                                                                                                 

kadi/commands/tests/test_commands.py ..............................................................s..............................                            [ 29%]
kadi/commands/tests/test_commands_v2.py ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss                              [ 57%]
kadi/commands/tests/test_filter_events.py ..                                                                                                                  [ 58%]
kadi/commands/tests/test_states.py .................................................x..........................                                               [ 82%]
kadi/commands/tests/test_validate.py ......................                                                                                                   [ 89%]
kadi/tests/test_events.py ..........                                                                                                                          [ 92%]
kadi/tests/test_occweb.py .......................                                                                                                             [100%]


================================================ 224 passed, 89 skipped, 1 xfailed, 2 warnings in 204.81s (0:03:24) =================================================
(ska3-latest) flame:kadi jean$ git rev-parse HEAD
fa6cb0ea348315faf23023ac32439d2913b88d49

Functional tests

No functional testing.

@jeanconn
Copy link
Copy Markdown
Contributor

Is this also ready for review?

@taldcroft
Copy link
Copy Markdown
Member Author

The actual code is ready. I'm going to add a quick test right now.

@taldcroft taldcroft requested a review from jeanconn March 24, 2026 17:35
@taldcroft
Copy link
Copy Markdown
Member Author

@jeanconn - ready for review!

@taldcroft taldcroft force-pushed the merge-obsid-obs-redux branch from e59cf8b to c874604 Compare March 25, 2026 09:38
Base automatically changed from select-scheduled-obsid to master March 25, 2026 12:55
@taldcroft
Copy link
Copy Markdown
Member Author

Rebased and retested.

Comment thread kadi/commands/observations.py
Comment thread kadi/commands/observations.py Outdated

# Now drop the current split observation that starts at the obsid change time.
# The `obs_stop` time (and all other params) are the same so just taking the
# first obs is all we need (no need to set obs_stop).
Copy link
Copy Markdown
Contributor

@jeanconn jeanconn Mar 25, 2026

Choose a reason for hiding this comment

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

I could use a quick explanation of this "no need to set obs_stop" part, as copilot and I were both confused by that one.

@taldcroft taldcroft merged commit e9791c2 into master Mar 25, 2026
4 checks passed
@taldcroft taldcroft deleted the merge-obsid-obs-redux branch March 25, 2026 18:31
@javierggt javierggt mentioned this pull request Apr 7, 2026
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.

2 participants