Conversation
compute_exposure_id and compute_ccdexposure_id generate unique numeric ids locally without a server round-trip, since summit_utils already depends on afw. compute_fixed_metadata_namespace is useful for generating query strings. insert_multiple allows many rows to be inserted at once (e.g. for many detectors in a focal plane).
mfisherlevine
left a comment
mfisherlevine
left a comment
There was a problem hiding this comment.
Looks good, minor comments about potentially moving the static methods. Just make sure to run the pre-commit hooks, guess you forced this to push, right?
| response.raise_for_status() | ||
| return response | ||
|
|
||
| @staticmethod |
There was a problem hiding this comment.
Could these static methods go to being free-floating functions? I think they'd be useful in other places as well as here, and they're fully generic. Given that, would you be up for putting them in utils.py in here, or some other file. so that they can be imported more naturally than putting them in the consdbClient? (and camelCasing while you're at it, given that?! 😬)
There was a problem hiding this comment.
The use of instrument.lower() is somewhat database-specific (most other places use the official capitalized name or a Python class name with its own distinct capitalization), but yes these could be moved.
| def compute_exposure_id(instrument: str, controller: str, day_obs: int, seq_num) -> int: | ||
| instrument = instrument.lower() | ||
| if instrument == "latiss": | ||
| from lsst.obs.lsst.translators import LatissTranslator |
There was a problem hiding this comment.
Are these imports particularly costly or something? I think everything in summit_utils has generally already imported the world, so if that's why these imports aren't at the top then I don't think it really matters. Was that the reason for putting them here?
There was a problem hiding this comment.
These are the only things in ConsDbClient that required anything stack-specific. Moving them out would mean that there's no need to hide them.
ktlim
force-pushed
the
tickets/DM-44387
branch
3 times, most recently
from
6e626bc to
b0539d6
Compare
2 participants
compute_exposure_id and compute_ccdexposure_id generate unique numeric ids locally without a server round-trip, since summit_utils already depends on afw.
compute_fixed_metadata_namespace is useful for generating query strings.
insert_multiple allows many rows to be inserted at once (e.g. for many detectors in a focal plane).