Skip to content

Conversation

@Gerenjie
Copy link
Contributor

@Gerenjie Gerenjie commented Nov 7, 2025

No description provided.

Copy link
Contributor

@erinleighh erinleighh left a comment

Choose a reason for hiding this comment

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

Looks good! Just some minor docstring changes. (Let's see if I did the multi-line review on Github correctly!)

Raises
------
ValueError
Raised if either objID is longer than 7 characters or flags is greater
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Raised if either objID is longer than 7 characters or flags is greater
Raised if either objID is shorter than 7 or longer than 8 characters or contains

------
ValueError
Raised if either objID is longer than 7 characters or flags is greater
than 255 or less than 0.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
than 255 or less than 0.
illegal objID characters.

Comment on lines 207 to 210
flags : `int`, optional
Eight free bits to enable future decoupling between Minor Planet Center
and Rubin. Zero by default, should not be changed unless we need to
move away from a 1:1 mapping with the MPC. Must be within [0, 255].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Comment on lines 251 to 253
flags : `int`
Rubin flags (not yet defined, but usable in case we decouple from MPC).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

@Gerenjie Gerenjie force-pushed the tickets/DM-53177 branch 3 times, most recently from 5d37dae to 6701121 Compare November 10, 2025 22:31
@Gerenjie Gerenjie merged commit 459e36d into main Nov 11, 2025
4 checks passed
@Gerenjie Gerenjie deleted the tickets/DM-53177 branch November 11, 2025 01:45
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.

3 participants