Skip to content
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

PREOPS-4838 Limiting the datetime selector’s range to those for which there are sky brightness files #67

Merged
merged 6 commits into from
Feb 7, 2024

Conversation

emanehab99
Copy link
Collaborator

Get the date range covered by SkyBrightness_Pre data files and limit date picker to this range so that user cannot select outside the values that have data

Copy link
Collaborator

@ehneilsen ehneilsen left a comment

Choose a reason for hiding this comment

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

This looks like the right basic approach, but I worry about getting the date range at the global level, rather than inside a method or function. The SkyModelPre class is is slow to instantiate, particularly when the argument to limit the initial load window is not set to something really short, and putting this at the global level means that importing the module is slowed down by this, even when the class never actually needs to be instantiated.

@emanehab99
Copy link
Collaborator Author

Thanks for the feedback @ehneilsen. I've wrapped the code in a function and rebased the branch

Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (8c7ea9e) 13.21% compared to head (8cb06dd) 13.15%.

Files Patch % Lines
...iew/app/scheduler_dashboard/scheduler_dashboard.py 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #67      +/-   ##
==========================================
- Coverage   13.21%   13.15%   -0.06%     
==========================================
  Files          43       43              
  Lines        3368     3375       +7     
  Branches      501      501              
==========================================
- Hits          445      444       -1     
- Misses       2914     2922       +8     
  Partials        9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@emanehab99
Copy link
Collaborator Author

@ehneilsen also fixed the tests

@emanehab99 emanehab99 merged commit 92b8cd3 into main Feb 7, 2024
8 of 9 checks passed
@rhiannonlynne rhiannonlynne deleted the tickets/PREOPS-4838 branch March 9, 2024 04:15
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