-
Notifications
You must be signed in to change notification settings - Fork 58
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
Update argument order in compose
#1521
Conversation
Cf. the discussion in Nemocas/AbstractAlgebra.jl#1025 |
Codecov Report
@@ Coverage Diff @@
## master #1521 +/- ##
==========================================
+ Coverage 82.75% 83.40% +0.65%
==========================================
Files 95 95
Lines 37186 39032 +1846
==========================================
+ Hits 30773 32556 +1783
- Misses 6413 6476 +63
... and 15 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
The breakage this introduces is of a worse kind than other such PRs, because instead of causing compile errors, it may result in some code actually continuing to work but return wrong results. I have no brilliant solution for this, though, other than perhaps it would be good to add If we had compose_left / compose_right (or whatever), then code could be converted to use them, making remaining uses of (Personally I think what I could work with best myself is to use Hrm. Sorry, no great solution from me here. We should still resolve this eventually, somehow! |
Could we perhaps keep the discussion at oscar-system/Oscar.jl#690? |
Making this a draft to prevent accidental merge |
too many conflicts. closing |
This updates all definitions and uses of
compose
in Nemo according to oscar-system/Oscar.jl#690 to followcompose(f,g) = g(f(x))
.This change is obviously breaking.