Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integer log is puzzled by Python int argument #21518

Closed
rwst opened this issue Sep 17, 2016 · 19 comments
Closed

Integer log is puzzled by Python int argument #21518

rwst opened this issue Sep 17, 2016 · 19 comments

Comments

@rwst
Copy link

rwst commented Sep 17, 2016

sage: ZZ(8).log(int(2))
log(8)/log(2)

Depends on #21517

Component: numerical

Author: Ralf Stephan

Branch/Commit: ab830ba

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/21518

@rwst rwst added this to the sage-7.4 milestone Sep 17, 2016
@rwst
Copy link
Author

rwst commented Sep 22, 2016

@rwst
Copy link
Author

rwst commented Sep 22, 2016

Dependencies: #21517

@rwst
Copy link
Author

rwst commented Sep 22, 2016

New commits:

de1acfa21517: handle ZZ.log(1/n)
3ce2cd221518: accept Python ints as log base to ZZ.log

@rwst
Copy link
Author

rwst commented Sep 22, 2016

Commit: 3ce2cd2

@rwst
Copy link
Author

rwst commented Sep 22, 2016

Author: Ralf Stephan

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2016

comment:3

Why not if type(m) == int:? Or perhaps, a more robust if m in ZZ: (or try: m = Integer(m))?

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2016

Reviewer: Travis Scrimshaw

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2016

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

79a5a4a21518: improve code

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2016

Changed commit from 3ce2cd2 to 79a5a4a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2016

Changed commit from 79a5a4a to 33a0de3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2016

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

4aba8a921517: avoid duplicate computations
33a0de3Merge branch 'u/rws/log_of_integer_to_base_1_n_can_have_exact_numeric_results' of trac.sagemath.org:sage into t/21518/integer_log_is_puzzled_by_python_int_argument

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2016

comment:7

Why is the if m is None: case in the try block? Could we just test that first?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 23, 2016

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

6e250f021518: reorder some lines

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 23, 2016

Changed commit from 33a0de3 to 6e250f0

@tscrim
Copy link
Collaborator

tscrim commented Sep 28, 2016

comment:9

One last thing, the if m in None: block is 1 over indented. Once you make this change, you can set a positive review on my behalf.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 29, 2016

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

ab830ba21518: cosmetics

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 29, 2016

Changed commit from 6e250f0 to ab830ba

@rwst
Copy link
Author

rwst commented Sep 29, 2016

comment:11

Thanks for the review.

@vbraun
Copy link
Member

vbraun commented Oct 5, 2016

Changed branch from u/rws/integer_log_is_puzzled_by_python_int_argument to ab830ba

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants