-
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
move code from Oscar.jl/experimental/GModule/Misc.jl
here
#1720
Conversation
The idea is to add this code to Nemo.jl, and then to remove it from Oscar.jl. Concerning the usefulness of this code (and tests to be added), I am not sure which vectors shall be supported by the new methods; vectors of integers are already addressed by other methods.
We need to adjust the signature here at least a little bit to not have something like oscar-system/Oscar.jl#3576 again. |
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 OK to me, although having a test case for both methods would be nice to ensure they are (and keep) working
I labeled this "breaking" for the same reason why Nemocas/AbstractAlgebra.jl#1675 is considered breaking. |
Yes, I will add tests. |
@ThomasBreuer I sent you an invite to this and the AA repository which hopefully will allow you to add labels, too |
* Bump dependencies * Remove moved aliases * Add `base_ring_type` methods * React to Nemocas/AbstractAlgebra.jl#1675 * React to Nemocas/Nemo.jl#1720 * Add QQBarField disambiguation method * Require AA 0.41.2 with `Generic.hom` fix * Add `base_ring(::QQAbField)` * Fix doctests * Fix booktests
The idea is to add this code to Nemo.jl, and then to remove it from Oscar.jl.
Concerning the usefulness of this code (and tests to be added), I am not sure which vectors shall be supported by the new methods; vectors of integers are already addressed by other methods.