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

@property methods doctests do not give correct line numbers #5046

Closed
asmeurer opened this issue May 28, 2010 · 15 comments
Closed

@property methods doctests do not give correct line numbers #5046

asmeurer opened this issue May 28, 2010 · 15 comments
Assignees
Labels
Bug Documentation imported Testing Related to the test runner. Do not use for test failures unless it relates to the test runner itself

Comments

@asmeurer
Copy link
Member

If you have something like:

class Whatever(object):
    ...
    @property
    def is_something(self):
        """
        Returns True if self has the something property.

        >>> from sympy import Whatever
        >>> Whatever().is_something
        True
        """
        return True   

The doctest will not be tested by bin/doctest.  It will, however, be caught by python -m doctest 
file.py.  

I am seeing if I can figure out how to fix it without having to resort to the workaround:

    _is_something_doc = """ docstring"""
    @property(doc=_is_something_doc)
    def is_something(self):
        return True

But if anyone has any hints or advice, it would be welcome :).

Original issue for #5046: http://code.google.com/p/sympy/issues/detail?id=1947
Original author: https://code.google.com/u/asmeurer@gmail.com/
Referenced issues: #4478
Original owner: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member Author

This is very perplexing.  Running a diff on runtests.py and the lib/python2.6/doctest.py file shows that very few 
things have changed between DocTestFinder and SymPyDocTestFinder, and none of them seem to be the 
problem.  

I think for now I am going to just verify my property doctests with the python doctest runner and put this on the 
back burner.  I do think it needs to be fixed before the next release though, especially if the @property doctests 
I am currently writing get pushed in by the next release :).

**Labels:** -Priority-Medium Priority-High Milestone-Release0.7.0  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c1
Original author: https://code.google.com/u/asmeurer@gmail.com/

@smichr
Copy link
Member

smichr commented Jun 1, 2010

Here is a note from runtests.py

    Modified from doctest's version by looking harder for code in the
    case that it looks like the the code comes from a different module.
    In the case of decorated functions (e.g. @vectorize) they appear
    to come from a different module (e.g. multidemensional) even though
    their code is not there.

See also the discussion at http://tinyurl.com/329fcb4

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c2
Original author: https://code.google.com/u/117933771799683895267/

@asmeurer
Copy link
Member Author

asmeurer commented Jun 1, 2010

Exactly!  Our doctest runner is supposed to be smarter about the @decorators, but the builtin doctest runner, 
from which 99% of the code of ours was copied, tests them correctly in the case of @property whereas ours 
doesn't (and none of the changes in a diff seem to indicate to me that they would cause this).

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c3
Original author: https://code.google.com/u/asmeurer@gmail.com/

@wxgeo
Copy link
Contributor

wxgeo commented Dec 20, 2010

Concerning doctests, I noticed the following (see issue 5022 , comment 30).

Such a docstring will not be tested by ./bin/doctest:

def my_func(*args, **kw):
   """Some text here.
          >>> 1 + 1
          3
      Some other text:
          >>> 1 + 2
          4
   """

but this one will be:

def my_func(*args, **kw):
   """Some text here.
          >>> 1 + 1
          3

      Some other text:
          >>> 1 + 2
          4
   """

and this one too:

def my_func(*args, **kw):
   """Some text here.
      >>> 1 + 1
      3
      Some other text:
      >>> 1 + 2
      4
   """

though I can't explain this...

Referenced issues: #5022
Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c4
Original author: https://code.google.com/u/117997262464115802198/

@asmeurer
Copy link
Member Author

asmeurer commented Jan 6, 2011

I figured it out.  

From the commit message:

    The problem is that we are sorting our doctests by line number (the
    Python doctest.py sorts by name) so when the _find_lineno function
    couldn't find the line number for the docstring, we were just skipping
    the docstring.

    But @property docstrings are not found by this function.  For now, we
    just replace the lineno with a dummy value in this case, but eventually
    we will need to write our own version that is smarter, because it prints
    an incorrect line number whenever a doctest fails in a @property
    method's docstring.

    This also fixes a handful of docstrings that are now caught.
    Fortunately, there wasn't really anything that was incorrect, just
    things that weren't imported in the doctest.

So I have a fix at https://github.com/sympy/sympy/pull/68 .  

Whenever this gets in, this issue should remain open (but the priority lowered), because I didn't fix the issue where failing @property doctests' line numbers are given incorrectly (we need to write or own _find_lineno).

**Labels:** -Priority-High Priority-Critical NeedsReview asmeurer  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c5
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member Author

asmeurer commented Jan 7, 2011

**Labels:** Testing  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c6
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member Author

This has been reviewed and pushed in.  See the pull request.

The issue is now just to write our own _find_lineno to handle @property doctests correctly.

**Summary:** @property methods doctests do not give correct line numbers  
**Labels:** -Priority-Critical -Milestone-Release0.7.0 -NeedsReview -asmeurer Priority-Medium  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c7
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member Author

After pushing this in, it revealed some test failures in secondquant:

________________________________________________________________________________
_______________ sympy.physics.secondquant.NO.has_q_annihilators ________________
File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/physics/secondquant.py", line 12, in sympy.physics.secondquant.NO.has_q_annihilators
Failed example:
    no_pq.has_q_annihilators
Exception raised:
    Traceback (most recent call last):
      File "/sw/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest sympy.physics.secondquant.NO.has_q_annihilators[4]>", line 1, in <module>
        no_pq.has_q_annihilators
    AttributeError: 'Mul' object has no attribute 'has_q_annihilators'
**********************************************************************
File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/physics/secondquant.py", line 15, in sympy.physics.secondquant.NO.has_q_annihilators
Failed example:
    no_pq.has_q_annihilators
Exception raised:
    Traceback (most recent call last):
      File "/sw/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest sympy.physics.secondquant.NO.has_q_annihilators[6]>", line 1, in <module>
        no_pq.has_q_annihilators
    AttributeError: 'Mul' object has no attribute 'has_q_annihilators'
________________________________________________________________________________
_________________ sympy.physics.secondquant.NO.has_q_creators __________________
File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/physics/secondquant.py", line 12, in sympy.physics.secondquant.NO.has_q_creators
Failed example:
    no_pq.has_q_creators
Exception raised:
    Traceback (most recent call last):
      File "/sw/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest sympy.physics.secondquant.NO.has_q_creators[4]>", line 1, in <module>
        no_pq.has_q_creators
    AttributeError: 'Mul' object has no attribute 'has_q_creators'
**********************************************************************
File "/Users/aaronmeurer/Documents/Python/sympy/sympy/sympy/physics/secondquant.py", line 15, in sympy.physics.secondquant.NO.has_q_creators
Failed example:
    no_pq.has_q_creators
Exception raised:
    Traceback (most recent call last):
      File "/sw/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest sympy.physics.secondquant.NO.has_q_creators[6]>", line 1, in <module>
        no_pq.has_q_creators
    AttributeError: 'Mul' object has no attribute 'has_q_creators'

Once again, Øyvind, I have absolutely no idea how to fix these.  (p.s., this commit has upped the number of doctests in the secondquant module tested by the runner from 18 to 45.  A pretty important fix, i'd say)

**Cc:** jorn.baayen fredrik.johansson  
**Labels:** -Priority-Medium Priority-Critical  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c8
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member Author

Sorry, CC'd wrong people.

**Cc:** -jorn.baayen -fredrik.johansson jensen.oyvind  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c9
Original author: https://code.google.com/u/asmeurer@gmail.com/

@jegerjensen
Copy link
Contributor

Aha! now I understand why there were so many failures in secondquant all of a sudden.  Great work.  I already prepared a patch https://github.com/sympy/sympy/pull/85

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c10
Original author: https://code.google.com/u/112426738183766874484/

@asmeurer
Copy link
Member Author

This was pushed in.

**Labels:** -Priority-Critical Priority-Medium  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c11
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member Author

**Status:** Valid  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c12
Original author: https://code.google.com/u/asmeurer@gmail.com/

@twmr
Copy link
Contributor

twmr commented Apr 5, 2013

see https://github.com/sympy/sympy/pull/1969

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c13
Original author: https://code.google.com/u/t.hisch@gmail.com/

@twmr
Copy link
Contributor

twmr commented Apr 14, 2013

please mark this as fixed

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c14
Original author: https://code.google.com/u/t.hisch@gmail.com/

@asmeurer
Copy link
Member Author

**Status:** Fixed  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1947#c15
Original author: https://code.google.com/u/asmeurer@gmail.com/

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Documentation imported Testing Related to the test runner. Do not use for test failures unless it relates to the test runner itself
Projects
None yet
Development

No branches or pull requests

5 participants