Opened 2 years ago

Closed 2 years ago

#29272 closed defect (fixed)

doctest in padic_lattice_element.py gives error in current develop branch

Reported by: gh-SagnikDey92 Owned by:
Priority: critical Milestone: sage-9.1
Component: padics Keywords:
Cc: kedlaya, edgarcosta, tscrim, roed, chapoton, caruso, saraedum, dimpase, dlucas, dkrenn, tmonteil, jsrn, gh-kliem Merged in:
Authors: Markus Wageringel Reviewers: Jonathan Kliem
Report Upstream: N/A Work issues:
Branch: 73e82fa (Commits, GitHub, GitLab) Commit: 73e82fa42ef973e42011badc54a988afb1ae1bf7
Dependencies: Stopgaps:

Status badges

Description

This is the error when doctesting in src/sage/rings/padics/padic_lattice_element.py:

File "src/sage/rings/padics/padic_lattice_element.py", line 19, in sage.rings.padics.padic_lattice_element
Failed example:
    R = ZpLF(2) # py3
Expected:
    doctest:...: FutureWarning: This class/method/function is marked as experimental. It, its functionality or its interface might change without a formal deprecation.
    See http://trac.sagemath.org/23505 for details.
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/rings/padics/padic_lattice_element.py", line 25, in sage.rings.padics.padic_lattice_element
Failed example:
    R = QpLC(2) # py3
Expected:
    doctest:...: FutureWarning: This class/method/function is marked as experimental. It, its functionality or its interface might change without a formal deprecation.
    See http://trac.sagemath.org/23505 for details.
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/rings/padics/padic_lattice_element.py", line 31, in sage.rings.padics.padic_lattice_element
Failed example:
    R = QpLF(2) # py3
Expected:
    doctest:...: FutureWarning: This class/method/function is marked as experimental. It, its functionality or its interface might change without a formal deprecation.
    See http://trac.sagemath.org/23505 for details.
Got:
    <BLANKLINE>
**********************************************************************
1 item had failures:
   3 of   5 in sage.rings.padics.padic_lattice_element
    [270 tests, 3 failures, 0.61 s]
----------------------------------------------------------------------
sage -t --warn-long 68.1 src/sage/rings/padics/padic_lattice_element.py  # 3 doctests failed
----------------------------------------------------------------------
Total time for all tests: 0.8 seconds
    cpu time: 0.6 seconds
    cumulative wall time: 0.6 seconds

Probably requires a simple change in the doctest.

Change History (15)

comment:1 Changed 2 years ago by mkoeppe

  • Cc kedlaya edgarcosta tscrim roed chapoton caruso saraedum added
  • Component changed from documentation to padics
  • Priority changed from minor to critical

comment:2 Changed 2 years ago by chapoton

see #20601 for some old related ticket

comment:3 Changed 2 years ago by mkoeppe

  • Branch set to u/mkoeppe/doctest_in_padic_lattice_element_py_gives_error_in_current_develop_branch

comment:4 Changed 2 years ago by mkoeppe

  • Commit set to 371c0c93285fcdc3447ef2b2f929b23ace63da18

Strange that this would only be noticed now?


New commits:

371c0c9Remove repeated FutureWarnings in doctests

comment:5 Changed 2 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Status changed from new to needs_review

comment:6 follow-up: Changed 2 years ago by roed

  • Status changed from needs_review to needs_info

With this branch, we're getting test failures on the patchbot where the FutureWarnings appear, so we clearly can't just remove them. I'm not sure why they appear sometime and not other times....

comment:7 Changed 2 years ago by mkoeppe

  • Cc dimpase added

Are the doctests in one block like this parallelized?

comment:8 Changed 2 years ago by roed

No, they should be run in series since they're within one block.

comment:9 Changed 2 years ago by roed

Another solution (that I should check with Xavier and Julian about) is to remove the experimental designation. We haven't worked on the lattice precision interface for a while, so maybe we should just think about what interface changes we want to make, make them, and then remove these warnings.

We've got a work week planned in mid April where we could look into this.

Last edited 2 years ago by roed (previous) (diff)

comment:10 in reply to: ↑ 6 Changed 2 years ago by dimpase

Replying to roed:

With this branch, we're getting test failures on the patchbot where the FutureWarnings appear, so we clearly can't just remove them. I'm not sure why they appear sometime and not other times....

what is so special about this bot? These test errors are happening most everywhere, I believe I wrote about it on some of your tickets, without any replies

comment:11 Changed 2 years ago by mkoeppe

  • Cc dlucas dkrenn tmonteil jsrn added

comment:12 Changed 2 years ago by gh-kliem

  • Cc gh-kliem added

comment:13 Changed 2 years ago by gh-mwageringel

  • Authors changed from Matthias Koeppe to Markus Wageringel
  • Branch changed from u/mkoeppe/doctest_in_padic_lattice_element_py_gives_error_in_current_develop_branch to u/gh-mwageringel/29272
  • Commit changed from 371c0c93285fcdc3447ef2b2f929b23ace63da18 to 73e82fa42ef973e42011badc54a988afb1ae1bf7
  • Status changed from needs_info to needs_review

This issue does not occur when running the long tests, but only when running the short tests, so this is not a problem on the patchbots. This might be a weird interaction between the doctest framework filtering duplicate warnings and the experimental decorator trying to avoid duplicate warnings.

This problem can be solved by reordering the doctests, so that the long test cases are executed after the short ones. Here is a branch that does this.


New commits:

73e82fa29272: reorder doctests to make long and short tests pass

comment:14 Changed 2 years ago by gh-kliem

  • Reviewers set to Jonathan Kliem
  • Status changed from needs_review to positive_review

LGTM.

(Lowering the stacklevel to 3 would have worked as well, but this is so much simpler).

comment:15 Changed 2 years ago by vbraun

  • Branch changed from u/gh-mwageringel/29272 to 73e82fa42ef973e42011badc54a988afb1ae1bf7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.