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

doctest in padic_lattice_element.py gives error in current develop branch #29272

Closed
SagnikDey92 mannequin opened this issue Mar 3, 2020 · 17 comments
Closed

doctest in padic_lattice_element.py gives error in current develop branch #29272

SagnikDey92 mannequin opened this issue Mar 3, 2020 · 17 comments

Comments

@SagnikDey92
Copy link
Mannequin

SagnikDey92 mannequin commented Mar 3, 2020

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.

CC: @kedlaya @edgarcosta @tscrim @roed314 @fchapoton @xcaruso @saraedum @dimpase @sagetrac-dlucas @dkrenn @sagetrac-tmonteil @johanrosenkilde @kliem

Component: padics

Author: Markus Wageringel

Branch/Commit: 73e82fa

Reviewer: Jonathan Kliem

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

@SagnikDey92 SagnikDey92 mannequin added this to the sage-9.1 milestone Mar 3, 2020
@fchapoton
Copy link
Contributor

comment:2

see #20601 for some old related ticket

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2020

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2020

comment:4

Strange that this would only be noticed now?


New commits:

371c0c9Remove repeated FutureWarnings in doctests

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2020

Commit: 371c0c9

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2020

Author: Matthias Koeppe

@roed314
Copy link
Contributor

roed314 commented Mar 22, 2020

comment:6

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....

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 24, 2020

comment:7

Are the doctests in one block like this parallelized?

@roed314
Copy link
Contributor

roed314 commented Mar 24, 2020

comment:8

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

@roed314
Copy link
Contributor

roed314 commented Mar 24, 2020

comment:9

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.

@dimpase
Copy link
Member

dimpase commented Mar 25, 2020

comment:10

Replying to @roed314:

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

@mwageringel
Copy link

comment:13

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

@mwageringel
Copy link

Changed commit from 371c0c9 to 73e82fa

@mwageringel
Copy link

Changed author from Matthias Koeppe to Markus Wageringel

@mwageringel
Copy link

@kliem
Copy link
Contributor

kliem commented Mar 31, 2020

comment:14

LGTM.

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

@kliem
Copy link
Contributor

kliem commented Mar 31, 2020

Reviewer: Jonathan Kliem

@vbraun
Copy link
Member

vbraun commented Apr 9, 2020

Changed branch from u/gh-mwageringel/29272 to 73e82fa

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

7 participants