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

Does not find examples when @property also has a @....setter defined #73

Closed
trappitsch opened this issue Jul 6, 2020 · 8 comments
Closed

Comments

@trappitsch
Copy link

Running Python 3.8.3 (set up with pyenv), xdoctest 0.12.0 (installed via pip install xdoctest[all]). In a class, when I have an @property defined, with a nice docstring, and then also define a setter, xdoctest does not see the example. Here's a minimum example:

class Test(object):
    @property
    def test(self):
        """
        Example:
            >>> ini = Test()
            >>> ini.test
            3.14
        """
        return 3.14

    @test.setter
    def test(self, s):
        pass

The output I receive when I run xdoctest test_min.py is:

=====================================
_  _ ___  ____ ____ ___ ____ ____ ___
 \/  |  \ |  | |     |  |___ [__   |
_/\_ |__/ |__| |___  |  |___ ___]  |

=====================================

Start doctest_module('xdoctest_min.py')
Listing tests
gathering tests
running 0 test(s)
============
===  in 0.00 seconds ===

If I remove the whole @test.setter method, the test succeeds properly. Any ideas on what might be going wrong here / what I'm missing?

@Erotemic
Copy link
Owner

Erotemic commented Jul 8, 2020

This is a bug I hadn't considered.

What's probably happening is that xdoctest uses static parsing to extract "callnames", which are the dotted strings that are used to identify code constructs that contain doctests. The doctests are then assigned to the "callnames".

The "callnames" of the getter is Test.test, but the setter callname is also "Test.test" (a callname is just a the function or classname or if its a method of a top-level class it is "{classname}.{funcname}". xdoctest doesn't parse anything under that top level.

The fix would be to find a way to differentiate the two methods, but I'm not sure what the best way to do that is. Maybe Test.test.fget and Test.test.fset make sense?

@Erotemic
Copy link
Owner

Erotemic commented Jul 9, 2020

@trappitsch I've created a potential fix for this in PR #75. Would you mind taking a look and providing feedback?

Instead of using .fget and .fset as suffixes, I opted to only suffix the setter with .fset and leave the callname of the getter unchanged. What do you think of this? Do you think both should be suffixed or only the setter? My feeling is that the getter more often stands alone so it might be better to omit the suffix in that case whereas I don't believe you can have a setter if you don't have a getter.

Note: the CI is currently failing due to a regression in pytest-cov, so the dashboards won't pass / I won't feel comfortable releasing until that is fixed.

@Erotemic Erotemic self-assigned this Jul 9, 2020
@trappitsch
Copy link
Author

@Erotemic Tested out PR #75 and it works great - it actually found an error in my docstring, and after fixing it passed all examples :)

I think you approach to only suffix the setter with .fset makes sense. In general, only the getter should have a docstring anyway, see here (assuming I understand this correctly):

If given, doc will be the docstring of the property attribute. Otherwise, the property will copy fget’s docstring (if it exists).

I surely use @property frequently without a setter at all, so I think your argument makes sense. Thanks for the fix!

@Erotemic
Copy link
Owner

Erotemic commented Jul 9, 2020

Thanks for that link. It wasn't immediately obvious to me that a setter wouldn't have a docstring at runtime. My implementation also neglects deleters (which would cause this bug to appear again), so I have to fix that.

It looks like its the case that setters can have docstrings, but they are clobbered when passed through the property(x).setter decorator and replaced with the docstring of x, (unless explicit docs are given?).

One of the side effects of doing all the parsing with static analysis is that I basically get to choose what is assigned to what, so in the current implementation xdoctest will actually think that there is a docstring for the setter and the getter. I do my best to try to be consistent with the dynamic analysis of docstrings that would have been done by the builtin doctest module, so perhaps the best method is to completely ignore any doctest in the setter / deleter.

I'm going to make those changes, and hopefully pytest-cov will be fixed soon so I can release the patch (or maybe I'll just monkey-patch pytest-cov on the dashboard for now, given that it's not needed by xdoctest at runtime).

@trappitsch
Copy link
Author

(unless explicit docs are given?)

I don't really understand what they mean in that part of that link I referenced before.

I did end up going a bit more down the rabbit hole and looked at some docstring standards. For example, pydocstyle supports by default PEP257, numpydoc, and the Google style guide. Not much in those guides though. The Google style guide has an example in section 2.3.14 where they only document the getter, but that's it for what I can see. Also, a bit further down they have a completely undocumented getter, so much for consistency ;)

The only real tangible hint, aside from blog posts and forums, that I could find on this is in the Sphinx examples for the Google and NumPy style guides. There it says (a bit hidden):

Properties created with the ``@property`` decorator should be documented in the property's getter method.

This also agrees with what you can find on stackoverflow, etc.

Long story short: there seems to be some consensus, however, nothing official. So it might be a good idea to allow the user to document getters, setters, and deleters after all? I'm fine with just allowing the getter too though, glad it's working now :) Thanks!

@Erotemic
Copy link
Owner

Erotemic commented Jul 10, 2020

The google example is consistent. Its ok to document getters, but you don't have to. Its also consistent in disallowing (by convention) documentation in setters / deleters. This is likely because those __doc__ attributes are not top-level accessible in the Python runtime state. (e.g when you do self.__class__.myprop.__doc__ you get the getter's doc by default, although it is possible to do self.__class__.myprop.fset.__doc__)

I did some digging of my own and came up with this example to disect the exact behavior of properties:

# This demonstrates dynamics of properties with clobbering any
# namespace variables, so its entirely clear what variables exist and how they
# are transformed as the property dectorators are applied
import pytest


def my_getter_func(self):
    " getter doc"
    print('call getter')
    return 'value'


def my_setter_func(self, value):
    " setter doc"
    print('call setter for value = {!r}'.format(value))


def my_deleter_func(self):
    " deleter doc"
    print('call deleter')


# Use the property decorator directly with normal call syntax
# Note properties --- like most other decorators --- return new function
# objects and do not change the underlying function object. Hense we can still
# print the original functions and see how they are assigned to the fset / fget
# / fdel attributes of the returned property object.
my_getter_prop = property(my_getter_func)
my_setter_prop = my_getter_prop.setter(my_setter_func)
my_deleter_prop = my_setter_prop.deleter(my_deleter_func)

print('my_getter_func = {!r}'.format(my_getter_func))
print('my_setter_func = {!r}'.format(my_setter_func))
print('my_deleter_func = {!r}'.format(my_deleter_func))

print('my_getter_prop = {!r}'.format(my_getter_prop))
print('my_setter_prop = {!r}'.format(my_setter_prop))
print('my_deleter_prop = {!r}'.format(my_deleter_prop))

print('my_getter_prop = {!r}'.format(my_getter_prop))
print('my_getter_prop.fget = {!r}'.format(my_getter_prop.fget))
print('my_getter_prop.fset = {!r}'.format(my_getter_prop.fset))
print('my_getter_prop.fdel = {!r}'.format(my_getter_prop.fdel))
print('my_getter_prop.__doc__ = {!r}'.format(my_getter_prop.__doc__))

print('my_setter_prop = {!r}'.format(my_setter_prop))
print('my_setter_prop.fget = {!r}'.format(my_setter_prop.fget))
print('my_setter_prop.fset = {!r}'.format(my_setter_prop.fset))
print('my_setter_prop.fdel = {!r}'.format(my_setter_prop.fdel))
print('my_setter_prop.__doc__ = {!r}'.format(my_setter_prop.__doc__))

print('my_deleter_prop = {!r}'.format(my_deleter_prop))
print('my_deleter_prop.fget = {!r}'.format(my_deleter_prop.fget))
print('my_deleter_prop.fset = {!r}'.format(my_deleter_prop.fset))
print('my_deleter_prop.fdel = {!r}'.format(my_deleter_prop.fdel))
print('my_deleter_prop.__doc__ = {!r}'.format(my_deleter_prop.__doc__))

# Note: each function has its own doc, but only the doc of the getter is stored

# my_getter_func          = <function my_getter_func at 0x7f2f14a6d700>
# my_setter_func          = <function my_setter_func at 0x7f2f14a6d0d0>
# my_deleter_func         = <function my_deleter_func at 0x7f2f14b88ee0>
# my_getter_prop          = <property object at 0x7f2f14c7b180>
# my_setter_prop          = <property object at 0x7f2f14d318b0>
# my_deleter_prop         = <property object at 0x7f2f14c81e00>
# my_getter_prop          = <property object at 0x7f2f14c7b180>
# my_getter_prop.fget     = <function my_getter_func at 0x7f2f14a6d700>
# my_getter_prop.fset     = None
# my_getter_prop.fdel     = None
# my_getter_prop.__doc__  = ' getter doc'
# my_setter_prop          = <property object at 0x7f2f14d318b0>
# my_setter_prop.fget     = <function my_getter_func at 0x7f2f14a6d700>
# my_setter_prop.fset     = <function my_setter_func at 0x7f2f14a6d0d0>
# my_setter_prop.fdel     = None
# my_setter_prop.__doc__  = ' getter doc'
# my_deleter_prop         = <property object at 0x7f2f14c81e00>
# my_deleter_prop.fget    = <function my_getter_func at 0x7f2f14a6d700>
# my_deleter_prop.fset    = <function my_setter_func at 0x7f2f14a6d0d0>
# my_deleter_prop.fdel    = <function my_deleter_func at 0x7f2f14b88ee0>
# my_deleter_prop.__doc__ = ' getter doc'

# Create an empty type
class Husk:
    pass

# Assigning properties to the class itself is equivalent to how they are
# normally defined in the scope of the class definition.
Husk.x = my_deleter_prop
Husk.y = my_setter_prop
Husk.z = my_getter_prop


# Creating an instance of the class will let us use our property variables
self = Husk()

# The "deleter" property has fget, fset, and fdel defined
self.x
self.x = 3
del self.x


# The "setter" property only had fget and fset defined
self.y
self.y = 3
with pytest.raises(AttributeError):
    del self.y


# The "getter" property only had fget defined
self = Husk()
self.z
with pytest.raises(AttributeError):
    self.z = 3
    del self.z

These tests demonstrate how the docstring of the property defaults to that of the getter. So, I'm fairly confident ignoring the setters / deleters is the right approach for consistency with the dynamic parser.

@trappitsch
Copy link
Author

Great, thanks for digging more into this and coming up with the example, that is very instructive and clearly supports your conclusion!

@Erotemic
Copy link
Owner

I downgraded pytest-cov in the CI scripts so the release scripts would pass. Xdoctest version 0.13.0 contains this fix and is now on pypi.

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

2 participants