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

py3: fix last 2 doctests in coding #26765

Closed
fchapoton opened this issue Nov 26, 2018 · 14 comments
Closed

py3: fix last 2 doctests in coding #26765

fchapoton opened this issue Nov 26, 2018 · 14 comments

Comments

@fchapoton
Copy link
Contributor

Component: python3

Author: Frédéric Chapoton

Branch/Commit: f34ad1c

Reviewer: Vincent Klein

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

@fchapoton fchapoton added this to the sage-8.5 milestone Nov 26, 2018
@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/26765

@fchapoton
Copy link
Contributor Author

Commit: f34ad1c

@fchapoton
Copy link
Contributor Author

New commits:

f34ad1cpy3: fix last doctests in coding

@vinklein
Copy link
Mannequin

vinklein mannequin commented Nov 26, 2018

comment:2

I think we should try to fix the root problem. Which seems to come from class Function_log2 from file sage.functions.log.
Note that there is other doctests falling because of this bug :

sage -t --long src/sage/functions/log.py
**********************************************************************
File "src/sage/functions/log.py", line 404, in sage.functions.log.log
Failed example:
    log(int(8),2)
Expected:
    3
Got:
    1
...

@fchapoton
Copy link
Contributor Author

comment:3

this is fixed in the next pynac, not yet released. Look for tickets about pynac..

@fchapoton
Copy link
Contributor Author

comment:4

#25979

@vinklein
Copy link
Mannequin

vinklein mannequin commented Nov 26, 2018

comment:5

Ok, but what should we do for the other cases then? (If we don't wait for pynac next release).
For File "src/sage/functions/log.py", line 404 case for example i see 3 possibilities :

  • Removing the doctest
  • Tag the doctest with # py2
  • Modify log function and convert the faulting parameter if we are in PY3 and if it's an int. If we do it this way it will also solve the error fixed by this ticket.

@vinklein
Copy link
Mannequin

vinklein mannequin commented Nov 26, 2018

comment:6

Or we can do a sage patch for pynac with pynac/pynac#336

@vinklein
Copy link
Mannequin

vinklein mannequin commented Nov 26, 2018

comment:7

My proposal for the patch #26770.

@fchapoton
Copy link
Contributor Author

comment:8

I agree that we need to fix the problem in general.

But could we please still validate the branch here ? This would finish the "coding" module.

@vinklein
Copy link
Mannequin

vinklein mannequin commented Nov 27, 2018

comment:9

I want to wait for Ralph Stephan answer about pynac release. At least today.
I think it would be better to avoid to convert into a Sage integer to get around the log bug if possible.
If we validate the branch right now and if #26770 is accepted or if a new pynac version is released in the next few days that means we should revert + return int(ZZ(codesize_upper_bound(n, d, q, algorithm=algorithm)).log(q)) to avoid unnecessary convertion.

@vinklein
Copy link
Mannequin

vinklein mannequin commented Nov 29, 2018

Reviewer: Vincent Klein

@vinklein
Copy link
Mannequin

vinklein mannequin commented Nov 29, 2018

comment:10

Ok, let's finish the coding module.

@vbraun
Copy link
Member

vbraun commented Nov 30, 2018

Changed branch from u/chapoton/26765 to f34ad1c

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

2 participants