Opened 3 years ago

Closed 2 years ago

#22103 closed defect (fixed)

Printing p-adic numbers

Reported by: caruso Owned by:
Priority: major Milestone: sage-7.5
Component: padics Keywords: padic printing, sd87
Cc: roed, saraedum Merged in:
Authors: Xavier Caruso Reviewers: David Roe, Adele Bourgeois
Report Upstream: N/A Work issues:
Branch: ce5ed91 (Commits) Commit: ce5ed91e17e9427caaef6c9fe09c8142c35cf772
Dependencies: Stopgaps:

Description

Currently, in the printing mode digits, the "initial" zeroes of a p-adic number are not displayed:

sage: R = Zp(5, print_mode='digits')
sage: R(89)
...324

It is a bit annoying because the precision does not appear on the printing. Even worse, when "initial" zeroes are located after the decimal mark, there appears as question mark whereas they are not unknown:

sage: x = R(89)/5^5; x
...?.??324
sage: x.precision_absolute()
15

I would suggest to change this behaviour and display all digits (including the initial zeroes) until the precision. Any thoughts?

Change History (17)

comment:1 Changed 3 years ago by roed

I'm happy to add zeros indicating the precision.

There's also a question of whether digits are should be printed in an opposite order, to align with the convention for real numbers where "smaller" digits appear further to the right. I'm comfortable with the current ordering, but others have suggested a change over the years.

comment:2 Changed 3 years ago by caruso

Personally, I do prefer the current ordering.

comment:3 Changed 3 years ago by chapoton

beware of current work going on in #22036 (python3 compatibility, needs review)

comment:4 Changed 2 years ago by caruso

  • Cc roed saraedum added; roes removed
  • Status changed from new to needs_review

Here is a proposal.

comment:5 Changed 2 years ago by caruso

  • Branch set to u/caruso/padic_printing

comment:6 Changed 2 years ago by git

  • Commit set to a2907e8643fc3db2c7cc53c9b281068431ab48b4

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

a2907e8Print significant 0's

comment:7 Changed 2 years ago by roed

  • Reviewers set to David Roe
  • Status changed from needs_review to needs_work

I'm happy with the general principle. A couple suggestions:

  • Why are there trailing zeros in the hunk -874,8 +874,24 @@ cdef class pAdicPrinter_class(SageObject)?
  • If they are supposed to be there, you should use alphabet[0] rather than "0".
  • Can you add some doctests showing the improved behavior?

comment:8 Changed 2 years ago by caruso

  • Status changed from needs_work to needs_review

The trailing zero is indeed not necessary, just for convenience. I however agree for using alphabet[0] instead of 0.

I've added doctests.

comment:9 Changed 2 years ago by git

  • Commit changed from a2907e8643fc3db2c7cc53c9b281068431ab48b4 to 1ded8ba5ec498b358ba89aa6ab6289a17a2d68f0

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

1ded8baSmall fixed + doctests

comment:10 Changed 2 years ago by git

  • Commit changed from 1ded8ba5ec498b358ba89aa6ab6289a17a2d68f0 to ce5ed91e17e9427caaef6c9fe09c8142c35cf772

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

ce5ed91Merge branch 'develop' into padic_printing

comment:11 Changed 2 years ago by roed

There's no problem merging this ticket with #20310. I'm rebuilding and running tests, but am about to get on a flight.

comment:12 Changed 2 years ago by roed

Okay, I had one test failure in src/sage/rings/padics/padic_ZZ_pX_CR_element.pyx after merging with #20310. I'm not sure if the problem is from changes introduced there.

We can keep talking about this in a week and a half, or you and Julian can resolve it.

comment:13 Changed 2 years ago by caruso

  • Keywords sd87 added

comment:14 Changed 2 years ago by abourgeois

  • Reviewers changed from David Roe to David Roe, Adele Bourgeois
  • Status changed from needs_review to positive_review

All doctests pass.

comment:15 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work

author name..

comment:16 Changed 2 years ago by caruso

  • Authors set to Xavier Caruso
  • Status changed from needs_work to positive_review

comment:17 Changed 2 years ago by vbraun

  • Branch changed from u/caruso/padic_printing to ce5ed91e17e9427caaef6c9fe09c8142c35cf772
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.