#27949 closed enhancement (fixed)
Pull out subfield() method from subfields() method.
Reported by: | klee | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-9.0 |
Component: | finite rings | Keywords: | |
Cc: | Merged in: | ||
Authors: | Kwankyu Lee | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | bd8ba10 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description
It is currently clumsy to create a subfield of a finite field using subfields() method.
sage: k = GF(2^21) sage: k.subfields(3) [(Finite Field in z3 of size 2^3, Ring morphism: From: Finite Field in z3 of size 2^3 To: Finite Field in z21 of size 2^21 Defn: z3 |--> z21^20 + z21^19 + z21^17 + z21^15 + z21^11 + z21^9 + z21^8 + z21^6 + z21^2)] sage: K, _ = k.subfields(3)[0] sage: K Finite Field in z3 of size 2^3
With the patch of this ticket, we have subfield() method such that
sage: k = GF(2^21) sage: k.subfield(3) Finite Field in z3 of size 2^3 sage: K = _ sage: k.coerce_map_from(K) Ring morphism: From: Finite Field in z3 of size 2^3 To: Finite Field in z21 of size 2^21 Defn: z3 |--> z21^20 + z21^19 + z21^17 + z21^15 + z21^11 + z21^9 + z21^8 + z21^6 + z21^2
Change History (17)
comment:1 Changed 3 years ago by
- Branch set to u/klee/27949
comment:2 Changed 3 years ago by
- Commit set to 87179a794b70ed68b22d5ad79d1b710948544701
comment:3 Changed 3 years ago by
- Status changed from new to needs_review
comment:4 Changed 3 years ago by
- Milestone changed from sage-8.8 to sage-8.9
Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).
comment:5 Changed 3 years ago by
- Commit changed from 87179a794b70ed68b22d5ad79d1b710948544701 to 30e85b3270327de07eab28d75780ba110c5d8a0e
Branch pushed to git repo; I updated commit sha1. New commits:
30e85b3 | Merge branch 'develop'
|
comment:6 Changed 3 years ago by
- Commit changed from 30e85b3270327de07eab28d75780ba110c5d8a0e to 147a412b31a73022220b267e0cc3a7e85ff1baf0
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
147a412 | Pull out subfield() from subfields()
|
comment:7 Changed 3 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
LGTM.
comment:8 follow-up: ↓ 9 Changed 3 years ago by
- Status changed from positive_review to needs_info
Sorry, I just noticed something:
return [self.subfields(m, name=name[m])[0] for m in divisors]
Couldn't this use your new subfield()
method?
comment:9 in reply to: ↑ 8 ; follow-up: ↓ 10 Changed 3 years ago by
Replying to tscrim:
Sorry, I just noticed something:
return [self.subfields(m, name=name[m])[0] for m in divisors]Couldn't this use your new
subfield()
method?
Then it would look like
- return [self.subfields(m, name=name[m])[0] for m in divisors] + + pairs = [] + for m in divisors: + K = self.subfield(m, name=name[m]) + pairs.append( (K, self.coerce_map_from(K)) ) + return pairs
It is somewhat duplicating the code above. Is this better?
comment:10 in reply to: ↑ 9 Changed 3 years ago by
Replying to klee:
Replying to tscrim:
Sorry, I just noticed something:
return [self.subfields(m, name=name[m])[0] for m in divisors]Couldn't this use your new
subfield()
method?Then it would look like
- return [self.subfields(m, name=name[m])[0] for m in divisors] + + pairs = [] + for m in divisors: + K = self.subfield(m, name=name[m]) + pairs.append( (K, self.coerce_map_from(K)) ) + return pairsIt is somewhat duplicating the code above. Is this better?
Maybe. It seems less wasteful in terms of object creation and it is not too much code duplication. Plus, the current version almost seems to to go against the new method; although I guess not exactly from what you're saying.
comment:11 Changed 3 years ago by
- Commit changed from 147a412b31a73022220b267e0cc3a7e85ff1baf0 to bd8ba100ef581c7a71a93a8e986db42721a3fb43
Branch pushed to git repo; I updated commit sha1. New commits:
bd8ba10 | Use subfield() in subfields() method
|
comment:12 Changed 3 years ago by
- Status changed from needs_info to needs_review
comment:14 Changed 3 years ago by
My pleasure. Thank you!
comment:15 Changed 3 years ago by
- Milestone changed from sage-8.9 to sage-9.0
moving milestone to 9.0 (after release of 8.9)
comment:16 Changed 3 years ago by
- Branch changed from u/klee/27949 to bd8ba100ef581c7a71a93a8e986db42721a3fb43
- Resolution set to fixed
- Status changed from positive_review to closed
comment:17 Changed 2 years ago by
- Commit bd8ba100ef581c7a71a93a8e986db42721a3fb43 deleted
Follow-up: #30171
Branch pushed to git repo; I updated commit sha1. New commits:
Pull out subfield() from subfields()