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.

Sign up for GitHub

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

Fix Stubgen's behavior for Instance Variables in C extensions #12524

Merged
merged 4 commits into from
Apr 11, 2022
Merged

Fix Stubgen's behavior for Instance Variables in C extensions #12524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc739fb
Fix Stubgen's behavior for Instance Variables in C extensions
Apr 5, 2022
245211c
Update stubgen test files
Apr 6, 2022
1a77ea9
Add test cases
Apr 7, 2022
34a2cdf
Fix test case
Apr 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions mypy/stubgenc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ def is_c_classmethod(obj: object) -> bool:


def is_c_property(obj: object) -> bool:
return inspect.isdatadescriptor(obj) and hasattr(obj, 'fget')
return inspect.isdatadescriptor(obj) or hasattr(obj, 'fget')


def is_c_property_readonly(prop: Any) -> bool:
return prop.fset is None
return hasattr(prop, 'fset') and prop.fset is None


def is_c_type(obj: object) -> bool:
Expand Down Expand Up @@ -287,6 +287,10 @@ def infer_prop_type(docstr: Optional[str]) -> Optional[str]:
else:
return None

# Ignore special properties/attributes.
if name.startswith('__') and name.endswith('__'):
return

inferred = infer_prop_type(getattr(obj, '__doc__', None))
if not inferred:
fget = getattr(obj, 'fget', None)
Expand Down
34 changes: 30 additions & 4 deletions mypy/test/teststubgen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
)
from mypy.stubutil import walk_packages, remove_misplaced_type_comments, common_dir_prefix
from mypy.stubgenc import (
generate_c_type_stub, infer_method_sig, generate_c_function_stub, generate_c_property_stub
generate_c_type_stub, infer_method_sig, generate_c_function_stub, generate_c_property_stub,
is_c_property_readonly
)
from mypy.stubdoc import (
parse_signature, parse_all_signatures, build_signature, find_unique_signatures,
Expand Down Expand Up @@ -868,9 +869,34 @@ def get_attribute(self) -> None:
pass
attribute = property(get_attribute, doc="")

output: List[str] = []
generate_c_property_stub('attribute', TestClass.attribute, [], [], output, readonly=True)
assert_equal(output, ['@property', 'def attribute(self) -> str: ...'])
readwrite_properties: List[str] = []
readonly_properties: List[str] = []
generate_c_property_stub('attribute', TestClass.attribute, [],
readwrite_properties, readonly_properties,
is_c_property_readonly(TestClass.attribute))
assert_equal(readwrite_properties, [])
assert_equal(readonly_properties, ['@property', 'def attribute(self) -> str: ...'])

def test_generate_c_property_with_rw_property(self) -> None:
class TestClass:
def __init__(self) -> None:
self._attribute = 0

@property
def attribute(self) -> int:
return self._attribute

@attribute.setter
def attribute(self, value: int) -> None:
self._attribute = value

readwrite_properties: List[str] = []
readonly_properties: List[str] = []
generate_c_property_stub("attribute", type(TestClass.attribute), [],
readwrite_properties, readonly_properties,
is_c_property_readonly(TestClass.attribute))
assert_equal(readwrite_properties, ['attribute: Any'])
assert_equal(readonly_properties, [])

def test_generate_c_type_with_single_arg_generic(self) -> None:
class TestClass:
Expand Down
4 changes: 0 additions & 4 deletions test-data/stubgen/pybind11_mypy_demo/basics.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ PI: float

class Point:
class AngleUnit:
__doc__: ClassVar[str] = ... # read-only
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since stubs for special dunder attributes is not expected to be generated, we remove these from the test files.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we had tests explicitly adding them, so I'd like to make sure these aren't important for pybind11 users.

@sizmailov added these tests in #9903; could you comment on whether removing these attributes from the docs is desirable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, annotation of most of dunder attributes makes a little sense and probably utilized only by few people if any, but I cannot speak for whole pybind11 community.
At the time I've added all the attributes to test just because it was generated by the stubgen and I saw no harm in leaving them.

It looks like dunder attributes hiding is orthogonal to the objectives of #12150.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like dunder attributes hiding is orthogonal to the objectives of #12150.

I agree on this, I had added this change to be in-line with stub generation logic for python files where we ignore special dunder methods (Ref).

We can also choose to not ignore dunder methods in generate_c_property_stub, in that case, we would have to update the below two tests since these will now generate stubs for dunder attributes:

@sizmailov Let me know if this sounds fine to you and I would then go ahead with the change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your change is OK as is. It is definitely an improvement.

My perception is that stubgen is still in the 0.x release state, therefore minor "breaking" changes are totally fine.

Last time I checked, stubgen had little or no configuration options, which results in many hardcoded decisions such as "no dunder attributes". But this problem is definitely out of the scope of this PR.

__members__: ClassVar[dict] = ... # read-only
__entries: ClassVar[dict] = ...
degree: ClassVar[Point.AngleUnit] = ...
radian: ClassVar[Point.AngleUnit] = ...
Expand All @@ -22,8 +20,6 @@ class Point:
def name(self) -> str: ...

class LengthUnit:
__doc__: ClassVar[str] = ... # read-only
__members__: ClassVar[dict] = ... # read-only
__entries: ClassVar[dict] = ...
inch: ClassVar[Point.LengthUnit] = ...
mm: ClassVar[Point.LengthUnit] = ...
Expand Down