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

FIX-#7113: Fix docstring overrides for subclasses. #7354

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 3 additions & 0 deletions modin/tests/config/docs_module/classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@
def apply():
"""This is a test of the documentation module for BasePandasDataSet.apply."""
return

def astype():
Dismissed Show dismissed Hide dismissed
"""This is a test of the documentation module for BasePandasDataSet.astype."""
18 changes: 18 additions & 0 deletions modin/tests/config/test_envvars.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# governing permissions and limitations under the License.

import os
import sys

import pandas
import pytest
Expand Down Expand Up @@ -96,6 +97,13 @@ def test_overrides(self):
assert BasePandasDataset.apply.__doc__ == (
"This is a test of the documentation module for BasePandasDataSet.apply."
)
# Test scenario 2 from https://github.com/modin-project/modin/issues/7113:
# We can correctly override the docstring for BasePandasDataset.astype,
# which is the same method as Series.astype.
assert pd.Series.astype is BasePandasDataset.astype
assert BasePandasDataset.astype.__doc__ == (
"This is a test of the documentation module for BasePandasDataSet.astype."
)
assert (
pd.DataFrame.apply.__doc__
== "This is a test of the documentation module for DataFrame."
Expand Down Expand Up @@ -130,6 +138,16 @@ def test_not_redefining_classes_modin_issue_7138(self):

assert pd.DataFrame is original_dataframe_class

def test_base_docstring_override_with_no_dataframe_or_series_class_issue_7113(
self,
):
# This test case tests scenario 1 from issue 7113.
sys.path.append(f"{os.path.dirname(__file__)}")
cfg.DocModule.put("docs_module_with_just_base")
assert BasePandasDataset.astype.__doc__ == (
"This is a test of the documentation module for BasePandasDataSet.astype."
)


@pytest.mark.skipif(cfg.Engine.get() != "Ray", reason="Ray specific test")
def test_ray_cluster_resources():
Expand Down
13 changes: 12 additions & 1 deletion modin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,13 @@ def _replace_doc(
# inherited docstrings.
_docstring_inheritance_calls: list[Callable[[str], None]] = []

# This is a set of (class, attribute_name) pairs whose docstrings we have
# already replaced since we last updated DocModule. Note that we don't store
# the attributes themselves since we replace property attributes instead of
# modifying them in place:
# https://github.com/modin-project/modin/blob/e9dbcc127913db77473a83936e8b6bb94ef84f0d/modin/utils.py#L353
_attributes_with_docstrings_replaced: set[tuple[type, str]] = set()


def _documentable_obj(obj: object) -> bool:
"""
Expand Down Expand Up @@ -417,6 +424,7 @@ def _update_inherited_docstrings(doc_module: DocModule) -> None:
doc_module : DocModule
The current DocModule.
"""
_attributes_with_docstrings_replaced.clear()
_doc_module = doc_module.get()
for doc_inheritance_call in _docstring_inheritance_calls:
doc_inheritance_call(doc_module=_doc_module) # type: ignore[call-arg]
Expand Down Expand Up @@ -488,7 +496,8 @@ def _inherit_docstrings_in_place(
if base is object:
continue
for attr, obj in base.__dict__.items():
if attr in seen:
# only replace docstrings once to prevent https://github.com/modin-project/modin/issues/7113
if attr in seen or (base, attr) in _attributes_with_docstrings_replaced:
continue
seen.add(attr)
# Try to get the attribute from the docs class first, then
Expand All @@ -511,6 +520,8 @@ def _inherit_docstrings_in_place(
attr_name=attr,
)

_attributes_with_docstrings_replaced.add((base, attr))


def _inherit_docstrings(
parent: object,
Expand Down
Loading