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:

Status badges

Description

when one knows if the ring is finite or not

Change History (12)

comment:1 Changed 5 years ago by chapoton

  • Branch set to u/chapoton/24408
  • Commit set to 44fc2aa71a21cf81727bc0047dd374dccee5a76e
  • Status changed from new to needs_review

New commits:

​44fc2aarefine the category of fraction fields

comment:2 Changed 5 years ago by chapoton

  • Cc fbissey embray added

green bot, please review

comment:3 Changed 5 years ago by tscrim

  • 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 vdelecroix

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 git

  • Commit changed from 44fc2aa71a21cf81727bc0047dd374dccee5a76e to d990d8064741232c1e4ee597a0362da3ead5571b

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

​d990d80trac 24408 step back and do not use characteristic (sigh)

comment:6 follow-up: Changed 5 years ago by chapoton

I have removed the try/except. As a consequence, fraction fields of polynomial rings do not get the new information that they are infinite.

Version 0, edited 5 years ago by chapoton (next)

comment:7 in reply to: ↑ 6 Changed 5 years ago by vdelecroix

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: Changed 5 years ago by chapoton

Indeed. But let us keep that for another ticket, no ?

comment:9 in reply to: ↑ 8 Changed 5 years ago by vdelecroix

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 vdelecroix

I am happy with this version. I let Travis apply the final sentence.

comment:11 Changed 5 years ago by tscrim

  • 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 vbraun

  • Branch changed from u/chapoton/24408 to d990d8064741232c1e4ee597a0362da3ead5571b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.