-
Notifications
You must be signed in to change notification settings - Fork 36
MOInput Type Stability #465
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
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Maybe add a @inferred test? |
Codecov Report
@@ Coverage Diff @@
## master #465 +/- ##
=======================================
Coverage 93.16% 93.16%
=======================================
Files 52 52
Lines 1259 1259
=======================================
Hits 1173 1173
Misses 86 86
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@theogf done! |
Thanks for the swift review @theogf -- will merge when CI passes |
@theogf looks like Others on 1.3 is failing on the doctests -- the formatting of the type printing appears to be different for that version. I don't know how tests were passing previously, because presumably this hasn't changed... 😬 Any thoughts? |
Probably the doctestfilter don't catch so many types or something? Didn't we mention dropping 1.3 anyways? |
Cool. I'm just going to drop CI for 1.3 |
Nvm, figured it out :) |
Summary
Makes the type of
out_dim
field ofMOInput
a type parameter ofMOInput
. This has been done in a non-breaking way.Proposed changes
See summary
What alternatives have you considered?
#405 consides an alternative approach in which the type is constrained to
Int
. This has the disadvantage of being more constrained, and constituting a break change. The potential advantage over the approach used here is slight changes in compile time, but my instinct is that these will be negligible.Breaking changes
None