#26765 closed enhancement (fixed)

py3: fix last 2 doctests in coding

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.5
Component: python3 Keywords:
Cc: Merged in:
Authors: Frédéric Chapoton Reviewers: Vincent Klein
Report Upstream: N/A Work issues:
Branch: f34ad1c (Commits) Commit: f34ad1caf5ad9c84e395212328ccd03134a02a6c
Dependencies: Stopgaps:

Description


Change History (11)

comment:1 Changed 13 months ago by chapoton

  • Branch set to u/chapoton/26765
  • Commit set to f34ad1caf5ad9c84e395212328ccd03134a02a6c
  • Status changed from new to needs_review

New commits:

f34ad1cpy3: fix last doctests in coding

comment:2 Changed 13 months ago by vklein

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
...
Last edited 13 months ago by vklein (previous) (diff)

comment:3 Changed 13 months ago by chapoton

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

comment:4 Changed 13 months ago by chapoton

comment:5 Changed 13 months ago by vklein

Ok, but what should we do for the other cases then (If we don't wait for pynac next release) ?. File "src/sage/functions/log.py", line 404 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.
Version 2, edited 13 months ago by vklein (previous) (next) (diff)

comment:6 Changed 13 months ago by vklein

Or we can do a sage patch for pynac with https://github.com/pynac/pynac/pull/336

comment:7 Changed 13 months ago by vklein

My proposal for the patch #26770.

comment:8 Changed 13 months ago by chapoton

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.

comment:9 Changed 13 months ago by vklein

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.

comment:10 Changed 13 months ago by vklein

  • Reviewers set to Vincent Klein
  • Status changed from needs_review to positive_review

Ok, let's finish the coding module.

comment:11 Changed 13 months ago by vbraun

  • Branch changed from u/chapoton/26765 to f34ad1caf5ad9c84e395212328ccd03134a02a6c
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.