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

tickets/PREOPS-5367: use target_note instead of target, and make schedview more robust to changes in the opsim database schema #100

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

ehneilsen
Copy link
Collaborator

No description provided.

@rhiannonlynne
Copy link
Member

This looks fine. Reran CI manually to ensure picking up rubin_scheduler v1.3.0 - https://github.com/lsst/schedview/actions/runs/10822668915

@rhiannonlynne
Copy link
Member

rhiannonlynne commented Sep 12, 2024

Ah - somewhere you are instantiating a basis function that has changed its API. Unit test still fails.
I am somewhat perplexed though, because I can't see where it's being called -- "SunHighLimitBasisFunction".

Copy link
Member

@rhiannonlynne rhiannonlynne left a comment

Choose a reason for hiding this comment

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

unit test failure needs attention

@ehneilsen ehneilsen force-pushed the tickets/PREOPS-5367 branch 2 times, most recently from 56e89b4 to f67ebeb Compare September 13, 2024 19:05
@ehneilsen
Copy link
Collaborator Author

Rebasing onto main after the merge of PREOPS-4102 seems to have solved the test failures. I don't know how this should up as something in SunHighLimitBasisFunction though.

@rhiannonlynne
Copy link
Member

This was probably that:
the pickle used SunHighLimitBasisFunction. In rubin_scheduler, we redirected SunHighLimitBasisFunction to CloseToTwilightBasisFunction (partly because of the confusion the testers had about the name, to make it clear that the goal was to get close to twilight). So the pickle was finding a valid class, but all of the attributes it restored were wrong (because they were actually for SunHighLimitBasisFunction) so it didn't actually work.
Now you have new pickles, it should be fine.

@rhiannonlynne rhiannonlynne self-requested a review September 13, 2024 20:35
@ehneilsen ehneilsen merged commit 1058087 into main Sep 13, 2024
7 checks passed
@ehneilsen ehneilsen deleted the tickets/PREOPS-5367 branch September 13, 2024 21:09
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