Opened 5 years ago

Closed 5 years ago

#20951 closed enhancement (fixed)

Fix Relative Field Extensions

Reported by: jlavauzelle Owned by:
Priority: major Milestone: sage-7.3
Component: coding theory Keywords:
Cc: dlucas Merged in:
Authors: Julien Lavauzelle Reviewers: David Lucas
Report Upstream: N/A Work issues:
Branch: 2510d2b (Commits, GitHub, GitLab) Commit: 2510d2b1312fb795aa59c67c90d4fd887598ddcf
Dependencies: Stopgaps:

Status badges

Description (last modified by jlavauzelle)

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

Change History (10)

comment:1 Changed 5 years ago by jlavauzelle

  • Branch set to u/jlavauzelle/fix_relative_field_extensions

comment:2 Changed 5 years ago by jlavauzelle

  • Authors set to Julien Lavauzelle
  • Branch u/jlavauzelle/fix_relative_field_extensions deleted
  • Component changed from PLEASE CHANGE to coding theory
  • Description modified (diff)
  • Status changed from new to needs_review
  • Type changed from PLEASE CHANGE to enhancement

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

comment:3 Changed 5 years ago by jlavauzelle

  • Branch set to u/jlavauzelle/fix_relative_field_extensions

comment:4 Changed 5 years ago by jlavauzelle

  • Cc dlucas added
  • Commit set to 819cbb970fd69974ac4940a1dc243a8fb568ba62

New commits:

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

comment:5 Changed 5 years ago by git

  • Commit changed from 819cbb970fd69974ac4940a1dc243a8fb568ba62 to 17bf28d79ec2461412f83397bf2930bd7e6bfd1c

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.

comment:6 Changed 5 years ago by jlavauzelle

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

comment:7 Changed 5 years ago by dlucas

  • Branch changed from u/jlavauzelle/fix_relative_field_extensions to u/dlucas/fix_relative_field_extensions

comment:8 Changed 5 years ago by dlucas

  • Commit changed from 17bf28d79ec2461412f83397bf2930bd7e6bfd1c to 2510d2b1312fb795aa59c67c90d4fd887598ddcf
  • Reviewers set to David Lucas

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

comment:9 Changed 5 years ago by jlavauzelle

  • Status changed from needs_review to positive_review

Hi David,

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

Thanks,

Julien

comment:10 Changed 5 years ago by vbraun

  • Branch changed from u/dlucas/fix_relative_field_extensions to 2510d2b1312fb795aa59c67c90d4fd887598ddcf
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.