-
Notifications
You must be signed in to change notification settings - Fork 651
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
FIX-#7263: Empty docstrings should not be inherited #7264
Conversation
Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>
@@ -434,6 +437,7 @@ def _documentable_obj(obj: object) -> bool: | |||
"""Check if `obj` docstring could be patched.""" | |||
return bool( | |||
callable(obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Classes are also callable, but we should not modify them. An important clarification - this only applies to classes that can be stored in the attributes of the class for which we are inheriting docstrings.
@@ -239,12 +239,12 @@ def wait(self): | |||
DaskWrapper.wait(self.list_of_blocks) | |||
|
|||
|
|||
@_inherit_docstrings(PandasOnDaskDataframeVirtualPartition.__init__) | |||
@_inherit_docstrings(PandasOnDaskDataframeVirtualPartition) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inheriting a docstring from a method for an entire class does not seem correct.
@@ -1118,7 +1118,7 @@ def merge_asof( | |||
tolerance=None, | |||
allow_exact_matches: bool = True, | |||
direction: str = "backward", | |||
): | |||
): # noqa: GL08 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, an empty string was assigned and the problem was not visible.
Now the question arises: why did the checker not pay attention to the empty string? This is most likely another bug :) In this situation, the successor does not override this function (PandasQueryCompiler.merge_asof is BaseQueryCompiler.merge_asof == True
), it turns out that we inherit an empty string and assign __doc_inherited__
label. And since the function is the same, we essentially disabled the check not only for the function that is docstring inherited, but also the function from which we took it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation :)
What do these changes do?
flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
git commit -s
added andpassingdocs/development/architecture.rst
is up-to-date