Opened 3 months ago

Closed 3 months ago

#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 3 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 3 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 3 months ago by vklein (previous) (diff)

comment:3 Changed 3 months ago by chapoton

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

comment:4 Changed 3 months ago by chapoton

comment:5 Changed 3 months ago by vklein

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

comment:6 Changed 3 months ago by vklein

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

comment:7 Changed 3 months ago by vklein

My proposal for the patch #26770.

comment:8 Changed 3 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 3 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 3 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 3 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.