Opened 5 years ago
Closed 5 years ago
#24408 closed enhancement (fixed)
refine the category of fraction field
Reported by: | chapoton | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | algebra | Keywords: | |
Cc: | jdemeyer, tscrim, fbissey, embray | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Travis Scrimshaw, Vincent Delecroix |
Report Upstream: | N/A | Work issues: | |
Branch: | d990d80 (Commits, GitHub, GitLab) | Commit: | d990d8064741232c1e4ee597a0362da3ead5571b |
Dependencies: | Stopgaps: |
Description
when one knows if the ring is finite or not
Change History (12)
comment:1 Changed 5 years ago by
- Branch set to u/chapoton/24408
- Commit set to 44fc2aa71a21cf81727bc0047dd374dccee5a76e
- Status changed from new to needs_review
comment:3 Changed 5 years ago by
- Reviewers set to Travis Scrimshaw
I'm not sure if this will be something that comes up. It is possible that something does not have characteristic
implemented, but it knows it is in Rings().Finite()
. So I feel like you should actually the characteristic test as the third try (not the second). Or should anything in Rings().Finite()
be able to compute its characteristic (say, by adding a method at the category level)?
comment:4 Changed 5 years ago by
I would even go further than Travis: it would be better to completely avoid calling the characteristic. That way
- it will not trigger an error (so that you can remove
try/except
) - faster, simpler
- avoid having a
FractionField
more aware of categories than the initial base ring
comment:5 Changed 5 years ago by
- Commit changed from 44fc2aa71a21cf81727bc0047dd374dccee5a76e to d990d8064741232c1e4ee597a0362da3ead5571b
Branch pushed to git repo; I updated commit sha1. New commits:
d990d80 | trac 24408 step back and do not use characteristic (sigh)
|
comment:6 follow-up: ↓ 7 Changed 5 years ago by
I have removed the try/except. As a consequence, fraction fields of polynomial rings do not get the new information that they are infinite.
comment:7 in reply to: ↑ 6 Changed 5 years ago by
Replying to chapoton:
I have removed the try/except. As a consequence, fraction fields of polynomial rings over QQ do not get the new information that they are infinite.
Better! One has to fix polynomial rings then (not their fraction fields).
sage: QQ['x'] in Sets().Infinite() False
comment:8 follow-up: ↓ 9 Changed 5 years ago by
Indeed. But let us keep that for another ticket, no ?
comment:9 in reply to: ↑ 8 Changed 5 years ago by
Replying to chapoton:
Indeed. But let us keep that for another ticket, no ?
Of course. I was just pointing the real problem.
comment:10 Changed 5 years ago by
I am happy with this version. I let Travis apply the final sentence.
comment:11 Changed 5 years ago by
- Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Vincent Delecroix
- Status changed from needs_review to positive_review
Yep, LGTM.
comment:12 Changed 5 years ago by
- Branch changed from u/chapoton/24408 to d990d8064741232c1e4ee597a0362da3ead5571b
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
refine the category of fraction fields