DM-36771: Bring Visit object in line with SAL spec #49
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kfindeisen
force-pushed
the
tickets/DM-36771
branch
from
22649a0 to
f4f0177
Compare
Such code is brittle to changes in the Visit API.
This brings the field in alignment with the SAL nextVisit spec.
This brings the fields in alignment with the SAL nextVisit spec. To support drop-in initialization of a Visit object from events, position must be a (non-hashable) list, not a tuple. However, all Visit objects are still effectively immutable, even if the class is not.
kfindeisen
force-pushed
the
tickets/DM-36771
branch
from
f4f0177 to
f4d09df
Compare
|
Tested with |
This brings the field in alignment with the SAL nextVisit spec.
This brings the field in alignment with the SAL nextVisit spec.
This brings the field in alignment with the SAL nextVisit spec. The activator code has been patched to handle nimages == 0, but the handler could be made more coherent.
This brings the field in alignment with the SAL nextVisit spec.
kfindeisen
force-pushed
the
tickets/DM-36771
branch
from
f4d09df to
2b552de
Compare
bsmartradio
approved these changes
Feb 10, 2023
The field is implemented as an IntEnum, since the actual message protocols treat it as a short. Visits may have topocentric coordinates instead of equatorial, so add a check to MiddlewareInterface that the coordinates really are ra/dec.
The field is implemented as an IntEnum, since the actual message protocols treat it as a short. Visits may have altaz or instrument coordinates rather than sky, so implement a check in MiddlewareInterface that the coordinates really are sky angle.
This change brings Visit in alignment with the SAL nextVisit spec. Note that salIndex is only present in the Kafka version of the message, and generally has a different value from scriptSalIndex.
kfindeisen
force-pushed
the
tickets/DM-36771
branch
from
2b552de to
97a6063
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR significantly rewrites the
Visitclass to have matching fields to the SALnextVisitspec (as modified for Kafka and Python). Since thenextVisitspec allows for states we hadn't previously considered in the prototype, I've also added checks or TODOs to the main codebase.