-
-
Notifications
You must be signed in to change notification settings - Fork 11
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 safe mode #55
Add safe mode #55
Conversation
Would it make sense for it to be a type parameter with two possible values, instead of a field? In other words, does it bring any benefit to have that information statically available? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 6 6
Lines 64 65 +1
=========================================
+ Hits 64 65 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Good question. It shouldn't matter for performance, because whether or not a rule operates in safe mode is baked in when you call On a practical level, (I think) adding type parameters would technically be breaking, so it would be good to avoid if possible. |
Okay then you can bump the minor version and I'll merge this |
Great, thanks! |
Tests only fail due to codecov throttling, merging now |
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Tapir.jl now has a "safe mode", in which lots of additional type checking is performed, in addition to some value checks. I would like to enable it by default (I'll be adding
@info
calls on the Tapir end to make it clear to users that they're using safe mode). This is to ensure that users are aware of this feature, which often makes debugging a lot easier, and can pick up on issues before they propagate to some arbitrary point further on in the code.Provided that people are happy with this, if someone could advise on the appropriate version bump for this package I would appreciate it.