-
-
Notifications
You must be signed in to change notification settings - Fork 33
Add _sym_uplo
to skip validation
#1441
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1441 +/- ##
=======================================
Coverage 93.89% 93.90%
=======================================
Files 34 34
Lines 15920 15925 +5
=======================================
+ Hits 14948 14954 +6
+ Misses 972 971 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
What about making this a one-line function?
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.
Can you please add tests that check the three possible outcomes of the previous sym_uplo
function to make sure (a) it's covered, (b) it doesn't change behavior, and finally (c) doesn't get removed by accident.
sym_uplo_unsafe
to skip validation_sym_uplo
to skip validation
@dkarrasch good to merge? |
In all the places where
sym_uplo
is used within this package, it is used in a context whereuplo in ('U', 'L')
, so the error path is never taken. We may therefore remove the error branch from the function and improve the effects inference.I've not removed the
sym_uplo
function, since this is used by some packages in the ecosystem (e.g.,JuMP
).