Opened 3 years ago
Closed 2 years ago
#22103 closed defect (fixed)
Printing padic numbers
Reported by:  caruso  Owned by:  

Priority:  major  Milestone:  sage7.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 padic 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
comment:2 Changed 3 years ago by
Personally, I do prefer the current ordering.
comment:3 Changed 3 years ago by
beware of current work going on in #22036 (python3 compatibility, needs review)
comment:4 Changed 2 years ago by
 Cc roed saraedum added; roes removed
 Status changed from new to needs_review
Here is a proposal.
comment:5 Changed 2 years ago by
 Branch set to u/caruso/padic_printing
comment:6 Changed 2 years ago by
 Commit set to a2907e8643fc3db2c7cc53c9b281068431ab48b4
Branch pushed to git repo; I updated commit sha1. New commits:
a2907e8  Print significant 0's

comment:7 Changed 2 years ago by
 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
 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
 Commit changed from a2907e8643fc3db2c7cc53c9b281068431ab48b4 to 1ded8ba5ec498b358ba89aa6ab6289a17a2d68f0
Branch pushed to git repo; I updated commit sha1. New commits:
1ded8ba  Small fixed + doctests

comment:10 Changed 2 years ago by
 Commit changed from 1ded8ba5ec498b358ba89aa6ab6289a17a2d68f0 to ce5ed91e17e9427caaef6c9fe09c8142c35cf772
Branch pushed to git repo; I updated commit sha1. New commits:
ce5ed91  Merge branch 'develop' into padic_printing

comment:11 Changed 2 years ago by
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
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
 Keywords sd87 added
comment:14 Changed 2 years ago by
 Reviewers changed from David Roe to David Roe, Adele Bourgeois
 Status changed from needs_review to positive_review
All doctests pass.
comment:16 Changed 2 years ago by
 Status changed from needs_work to positive_review
comment:17 Changed 2 years ago by
 Branch changed from u/caruso/padic_printing to ce5ed91e17e9427caaef6c9fe09c8142c35cf772
 Resolution set to fixed
 Status changed from positive_review to closed
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.