Opened 4 years ago
Closed 4 years ago
#15373 closed enhancement (fixed)
Implement global_height for Integers
Reported by: | paulfili | Owned by: | paulfili |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.13 |
Component: | algebraic geometry | Keywords: | sage-days55 |
Cc: | Merged in: | sage-5.13.beta3 | |
Authors: | Paul Fili | Reviewers: | Adam Towsley |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
QQ(5).global_height() works but ZZ(5).global_height() does not.
Implement global_height for ZZ elements.
Attachments (1)
Change History (18)
comment:1 Changed 4 years ago by
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
- Keywords sage-days55 added; sage-dasy55 removed
comment:3 Changed 4 years ago by
comment:4 Changed 4 years ago by
- Status changed from needs_review to positive_review
comment:5 Changed 4 years ago by
- Status changed from positive_review to needs_work
Please change the commit message to something meaningful and fill in the Reviewer field with your real name.
comment:6 Changed 4 years ago by
- Reviewers set to Adam Towsley
comment:7 Changed 4 years ago by
- Status changed from needs_work to needs_review
Thanks, I have changed the commit message.
comment:8 Changed 4 years ago by
- Status changed from needs_review to positive_review
Everything still works.
comment:9 Changed 4 years ago by
- Status changed from positive_review to needs_work
- Work issues set to docstring
The indentation of the documentation should be
""" Documentation here """
without the extra indent.
comment:10 Changed 4 years ago by
- Status changed from needs_work to needs_review
I have fixed the indentation in the documentation.
comment:11 Changed 4 years ago by
- Status changed from needs_review to needs_work
The indentation is still wrong. It should be:
def global_height(self, prec=None): r""" Returns the absolute logarithmic height of this rational integer. INPUT: - ``prec`` (int) -- desired floating point precision (default: default RealField precision). OUTPUT: (real) The absolute logarithmic height of this rational integer. ALGORITHM: The height of the integer `n` is `\log |n|`. EXAMPLES:: sage: ZZ(5).global_height() 1.60943791243410 sage: ZZ(-5).global_height(prec=100) 1.6094379124341003746007593332 # This is log(5): sage: exp(_) 5.0000000000000000000000000000 """
comment:12 Changed 4 years ago by
- Status changed from needs_work to needs_review
Thanks for your patience. I've uploaded a new version with the indentation as you specified.
comment:13 Changed 4 years ago by
I get the following error with the patch...
sage -t --long integer.pyx ********************************************************************** File "integer.pyx", line 4150, in sage.rings.integer.Integer.global_height Failed example: ZZ(-5).global_height(prec=100) Expected: 1.6094379124341003746007593332 # This is log(5): Got: 1.6094379124341003746007593332 ********************************************************************** 1 item had failures: 1 of 4 in sage.rings.integer.Integer.global_height [916 tests, 1 failure, 5.69 s] ---------------------------------------------------------------------- sage -t --long integer.pyx # 1 doctest failed ---------------------------------------------------------------------- Total time for all tests: 6.0 seconds cpu time: 4.5 seconds cumulative wall time: 5.7 seconds
It's the only error I get, but I don't see why it appears. Since the two values it prints (expected and got) are the same.
comment:14 Changed 4 years ago by
Perhaps the comment on the next line threw it off. I've removed it and put a better example. Should work now.
comment:15 Changed 4 years ago by
- Status changed from needs_review to positive_review
Yes, that did it. Everything works now.
comment:16 Changed 4 years ago by
- Work issues docstring deleted
comment:17 Changed 4 years ago by
- Merged in set to sage-5.13.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
It passed the doc test and the long test.
Functionality still works.