-
Notifications
You must be signed in to change notification settings - Fork 3
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
Use cached property in commands validation #295
Conversation
1b670dc
to
d9b77d5
Compare
d9b77d5
to
97e6c8b
Compare
97e6c8b
to
87bf871
Compare
f"Fetching telemetry for {self.msids} between {self.start.date} and" | ||
f" {self.stop.date}" | ||
) | ||
_tlm = get_telem_table(self.msids, self.start, self.stop) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're moving away from using attributes directly to store these (viaCLS._attribute_names
) it feels a little odd to bother with the underscore names, as we tend not to use them generally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to not overload the method function itself. I.e. inside def tlm(self)
, tlm
is defined as the unbound method. A type checker or IDE will notice stuff like this. I could call everything out
but I think the current naming somewhat hints at the property-ness of the methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I wasn't suggesting tlm
as the replacement but fine to leave these with the current naming hinting at the property-ness of the internals.
Description
This just moves to code to use a nicer pattern for cached properties. This required a slight change in when the exclude intervals are fetched from Google sheets.
The PR also makes one small out-of-scope change to remove the unused
update_tlm()
base method. That originally served a purpose but is no longer used.Interface impacts
Testing
Used local ska3-performance installation.
Unit tests
Independent check of unit tests by Jean
Functional tests
No functional testing.