-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add test of optional ACA limits #385
Conversation
@jeanconn - please approve if you are good with this. |
I eye-balled the new git and model fetching and manipulation but haven't had a chance to do review on this yet. |
Ah OK, I misinterpreted the scope of "This seems fine". No problem. |
_test_optional_penalty_limit() | ||
finally: | ||
# Reset ACA limits characteristics that get set above | ||
ACA._set_aca_limits() |
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.
OK, so real testing question -- this makes sense but would there be other advantages to using a fixture to do this instead? Or have you not used a fixture because you prefer to explicitly test that ACA._set_aca_limits gets back to the 3.48 values?
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.
I wasn't exactly sure how to do this with a fixture and decided this was good enough.
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.
Looks good. The business with manually running ACA._set_aca_limits() to override the proseco "cache" and lazy load for these attributes reminded me... should proseco also be using @chandra_models.chandra_models_cache to get the model?
Good point. I think something like this can work and be a good thing. Next PR... #386 |
Description
This adds a unit test that verifiers the functionality in #384.
As a small out-of-scope change, move to
matplotlib.pyplot.switch_backend
to avoid the linting problem with imports after code.Interface impacts
None.
Testing
Unit tests
Independent check of unit tests by Jean
Functional tests
No functional testing.