Opened 10 years ago

Closed 9 years ago

#13539 closed enhancement (fixed)

Add inverse_of_unit() for padics

Reported by: saraedum Owned by: roed
Priority: minor Milestone: sage-5.8
Component: padics Keywords:
Cc: Merged in: sage-5.8.beta2
Authors: Julian Rueth Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #13600 Stopgaps:

Status badges

Description (last modified by saraedum)

Currently, the padics lack an inverse_of_unit() method:

sage: (-1).inverse_of_unit()
-1
sage: ZpCA(3)(-1).inverse_of_unit()
AttributeError

The drawback with simply using ~ is that it puts the result in the fraction field and it can be annoying to always convert it back to the original ring when implementing general algorithms for all padics rings and fields:

sage: t = ZpCA(3,2)(-1)
sage: t
2 + 2*3 + O(3^2)
sage: t.parent()
3-adic Ring with capped absolute precision 2
sage: ~t
2 + 2*3 + O(3^2)
sage: (~t).parent()
3-adic Field with capped relative precision 2

The attached patch implements a method inverse_of_unit() and fixes a conversion error that came up when testing it.

Attachments (1)

trac_13539.patch (5.3 KB) - added by saraedum 10 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by saraedum

  • Description modified (diff)

comment:2 Changed 10 years ago by saraedum

  • Status changed from new to needs_review

comment:3 Changed 10 years ago by roed

  • Status changed from needs_review to needs_work

I'm happy with the changes to LocalGenericElement. There are changes to pAdicZZpXCAElement though that I think make the code less readable (and actually change the functionality a bit). Were those intended?

comment:4 Changed 10 years ago by saraedum

  • Dependencies set to #13600

You're right. I didn't understand these changes myself anymore now. I tried to put them into a separate ticket and turned them into something that is hopefully more readable #13600.

comment:5 Changed 10 years ago by roed

Cool. Once you update the patch here and it passes doctests I can give it a positive review

comment:6 Changed 10 years ago by saraedum

  • Status changed from needs_work to needs_review

I removed the pieces that are now in #13600. I also added a copyright notice since it was missing (I took the years from the repo history).

comment:7 Changed 10 years ago by roed

Your apply failed against 5.4.rc1 according to patchbot. I'm a bit confused though, since I'm succeeding.... If you don't know what's going on I'll take a look later.

Changed 10 years ago by saraedum

comment:8 Changed 10 years ago by roed

  • Status changed from needs_review to positive_review

Okay, patchbot is succeeding now, so I'll give this a positive review.

comment:9 Changed 10 years ago by roed

  • Reviewers set to David Roe

comment:10 Changed 10 years ago by jdemeyer

  • Milestone changed from sage-5.4 to sage-pending

comment:11 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.8

comment:12 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.8.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.