Opened 6 years ago
Closed 5 years ago
#17569 closed enhancement (fixed)
Allow creating finite fields without a variable name
Reported by:  pbruin  Owned by:  

Priority:  minor  Milestone:  sage7.1 
Component:  finite rings  Keywords:  
Cc:  ncohen, vdelecroix, jpflori, dimpase, bruno, jdemeyer, vbraun  Merged in:  
Authors:  David Roe  Reviewers:  Volker Braun 
Report Upstream:  N/A  Work issues:  
Branch:  2a92ef3 (Commits)  Commit:  2a92ef3568d32310b1b63d99dc54c313f58d671e 
Dependencies:  Stopgaps: 
Description
It should be possible to use FiniteField(p^n)
(or FiniteField(p, n)
, see #17568) without a name
argument to create the subfield of FiniteField(p).algebraic_closure()
of cardinality p^n
. This would mean
GF(p, n) = GF(p^n) = GF(p).algebraic_closure().subfield(n)[0]
Discussion on sagedevel: https://groups.google.com/forum/#!topic/sagedevel/LlstHp950uo
Change History (22)
comment:1 Changed 5 years ago by
 Branch set to u/roed/allow_creating_finite_fields_without_a_variable_name
comment:2 Changed 5 years ago by
 Commit set to a56ec7453875e5caeface30c5c7689402cf04fba
 Milestone changed from sage6.5 to sage7.0
 Status changed from new to needs_review
comment:3 Changed 5 years ago by
 Milestone changed from sage7.0 to sage7.1
 Status changed from needs_review to needs_work
Why did you remove the whole # The following is a temporary solution...
block? Do you want conway
and prefix
to continue to be supported?
 If no => deprecate it.
 If yes => document it properly (i.e. move the comment you just removed to the actual docstring) and add some doctests like
sage: GF(3^10, conway=True, prefix='z') is GF(3^10) sage: GF(3^10, conway=True, prefix='foo')
comment:4 followup: ↓ 5 Changed 5 years ago by
Hello !
I am affraid that I am not sufficiently skilled to review this branch, but I added a small commit at public/17569 which fixes a couple of typos and adds a link.
Nathann
comment:5 in reply to: ↑ 4 Changed 5 years ago by
Replying to ncohen:
Hello !
I am affraid that I am not sufficiently skilled to review this branch, but I added a small commit at public/17569 which fixes a couple of typos and adds a link.
Great. I have to go teach now, but I'll take a look in the next day or so.
Nathann
comment:6 Changed 5 years ago by
 Commit changed from a56ec7453875e5caeface30c5c7689402cf04fba to 2fcbc08667d7048d1f9a2691c8e4e9a2eff66001
comment:7 Changed 5 years ago by
 Status changed from needs_work to needs_review
Okay, I've added some documentation about how the conway and prefix keywords are handled.
comment:8 Changed 5 years ago by
What's the point?
This keyword argument is now silently ignored.
Sounds like it should be deprecated...
comment:9 followups: ↓ 10 ↓ 11 ↓ 12 Changed 5 years ago by
Currently, you can feed random keyword arguments to most (all?) of the finite field constructors and they'll have no effect.
sage: GF(27,'a',blah=True) Finite Field in a of size 3^3
I don't see a problem in having conway
behave in the same manner. Why raise a deprecation warning when the code can just keep working without any change?
comment:10 in reply to: ↑ 9 Changed 5 years ago by
Replying to roed:
Currently, you can feed random keyword arguments to most (all?) of the finite field constructors and they'll have no effect.
sage: GF(27,'a',blah=True) Finite Field in a of size 3^3I don't see a problem in having
conway
behave in the same manner. Why raise a deprecation warning when the code can just keep working without any change?
You are effecting a change in the behaviour of the code. E.g.
sage: GF(27,'a',conway=True)
currently throws an exception, and this is will no longer be the case. (Perhaps one can come up with a more interesting example, but that's beside the point).
comment:11 in reply to: ↑ 9 Changed 5 years ago by
Replying to roed:
Currently, you can feed random keyword arguments to most (all?) of the finite field constructors and they'll have no effect.
These uselessbutaccepted arguments are a plague. There was a wealth of them in many 'plot' functions at a time: you tried to give a color to something, the argument 'color="red"' was accepted but nothing happened.
The most common way in which they are a pain are typos:
sage: GF(16,'x',checkirreducible=True) Finite Field in x of size 2^4
The sideeffect of not having a '_' in check_irreducible
is that... It is ignored. And of course no exception to tell you.
About having an effect, here is one (thanks to UniqueRepresentation
, I expect)
sage: GF(27,'a') == GF(27,'a') True sage: GF(27,'a',whatever=158) == GF(27,'a') False sage: GF(27,'a')(1) + GF(27,'a')(1) 2 sage: GF(27,'a',whatever=158)(1) + GF(27,'a')(1) TypeError: unsupported operand parent(s) for '+': 'Finite Field in a of size 3^3' and 'Finite Field in a of size 3^3'
Nathann
comment:12 in reply to: ↑ 9 Changed 5 years ago by
Replying to roed:
Currently, you can feed random keyword arguments to most (all?) of the finite field constructors and they'll have no effect.
That's certainly a serious bug, but outside the scope of this ticket.
Still, I think that conway
needs to be deprecated because it used to do something. Users should be made aware that the conway
argument is no longer needed. Something like this should work:
if 'conway' in kwds: del kwds['conway'] from sage.misc.superseded import deprecation deprecation(17569, "the 'conway' argument is deprecated, pseudoconway polynomials are now used by default if no variable name is given")
comment:13 Changed 5 years ago by
 Status changed from needs_review to needs_work
comment:14 Changed 5 years ago by
 Commit changed from 2fcbc08667d7048d1f9a2691c8e4e9a2eff66001 to 9d7c717c1afbb0d7a7abd1ad93c0c3d63043e6ad
Branch pushed to git repo; I updated commit sha1. New commits:
9d7c717  Deprecating conway keyword argument to finite field constructor

comment:15 Changed 5 years ago by
Conflict due to #19941. Thanks Nathann.
comment:16 Changed 5 years ago by
Oh true, the rebase. Right. Give me a second.
comment:17 Changed 5 years ago by
Helloooooo !
As expected, it was a nightmare :P
You will find the rebased branch at u/ncohen/17569
.
Nathann
comment:18 Changed 5 years ago by
 Branch changed from u/roed/allow_creating_finite_fields_without_a_variable_name to u/ncohen/17569
 Commit changed from 9d7c717c1afbb0d7a7abd1ad93c0c3d63043e6ad to 69ecf50c5fc1a7ad794ad7e3e40f0493314b5d04
 Status changed from needs_work to needs_review
Cool. Thanks for the rebase.
comment:19 Changed 5 years ago by
It was just the worst amount of 'bad'. Too bad to be 'easy', and not sufficiently bad to force me to learn how to merge two branches which work on a renamed file, in general.
Well. Next time :P
comment:20 Changed 5 years ago by
 Branch changed from u/ncohen/17569 to u/roed/17569
comment:21 Changed 5 years ago by
 Commit changed from 69ecf50c5fc1a7ad794ad7e3e40f0493314b5d04 to 2a92ef3568d32310b1b63d99dc54c313f58d671e
 Reviewers set to Volker Braun
 Status changed from needs_review to positive_review
New commits:
2a92ef3  Fixing doctest errors from conway kwd deprecation

comment:22 Changed 5 years ago by
 Branch changed from u/roed/17569 to 2a92ef3568d32310b1b63d99dc54c313f58d671e
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Enabling GF(p^n) with no variable name given