Skip to content
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

Fix Relative Field Extensions #20951

Closed
jlavauzelle opened this issue Jul 6, 2016 · 17 comments
Closed

Fix Relative Field Extensions #20951

jlavauzelle opened this issue Jul 6, 2016 · 17 comments

Comments

@jlavauzelle
Copy link
Contributor

This tickets aims at fixing some minors bugs in the experimental class RelativeFiniteFieldExtension.

CC: @sagetrac-dlucas

Component: coding theory

Author: Julien Lavauzelle

Branch/Commit: 2510d2b

Reviewer: David Lucas

Issue created by migration from https://trac.sagemath.org/ticket/20951

@jlavauzelle jlavauzelle added this to the sage-7.3 milestone Jul 6, 2016
@jlavauzelle
Copy link
Contributor Author

@jlavauzelle

This comment has been minimized.

@jlavauzelle
Copy link
Contributor Author

Author: Julien Lavauzelle

@jlavauzelle
Copy link
Contributor Author

Changed branch from u/jlavauzelle/fix_relative_field_extensions to none

@jlavauzelle
Copy link
Contributor Author

comment:2

Hi,

I fixed ambiguity in the documentation of some functions. I also fixed a minor bug in the method relative_field_representation.

I added a function which basically invert the relative field embedding (I mean, given an absolute field element x which actually lies in the relative field, the function cast x into the relative field).

Didn't check the other methods.

Open for review.

Julien

@jlavauzelle
Copy link
Contributor Author

@jlavauzelle
Copy link
Contributor Author

New commits:

f77da55Fixed the doc. Fixed the case s==1 in relative_field_representation().
819cbb9Add the inverse function of the relative field embedding.

@jlavauzelle
Copy link
Contributor Author

Commit: 819cbb9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2016

Changed commit from 819cbb9 to 17bf28d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

17bf28dReplaced "power" by "degree" when dealing with the extension degree of a field. Added an enxtension_degree() method.

@jlavauzelle
Copy link
Contributor Author

comment:6

Hi,

I reviewed the rest of the class and modified some names (basically 'degree' instead of 'power'). I also added a function to give the extension degree between the medium and the big field, as we often need it.

Open for review.

Julien

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Jul 8, 2016

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Jul 8, 2016

Changed commit from 17bf28d to 2510d2b

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Jul 8, 2016

Reviewer: David Lucas

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Jul 8, 2016

comment:8

Hello,

I made two small changes:

  • There was an indentation error in the docstring of cast_into_relative_field, which I fixed.
  • In Python, it's not necessary to write if (check):, if check: is enough :). Thus I removed the unnecessary parenthesis.

Otherwise, it's good to go! If you agree with my tiny changes, you can set it to positive_review.
Thanks for your fixes.

BTW, the patchbot doctests error due to guava.py have been fixed in #20952.

Best,

David


New commits:

6ace61bFixed small bug in documentation
2510d2bRemoved unnecessary parenthesis

@jlavauzelle
Copy link
Contributor Author

comment:9

Hi David,

That's ok for me. I put it in positive review.

Thanks,

Julien

@vbraun
Copy link
Member

vbraun commented Jul 9, 2016

Changed branch from u/dlucas/fix_relative_field_extensions to 2510d2b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants