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 all commits
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 @@ class BasePandasDataset:
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."""
16 changes: 16 additions & 0 deletions modin/tests/config/docs_module_with_just_base/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Licensed to Modin Development Team under one or more contributor license agreements.
# See the NOTICE file distributed with this work for additional information regarding
# copyright ownership. The Modin Development Team licenses this file to you under the
# Apache License, Version 2.0 (the "License"); you may not use this file except in
# compliance with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software distributed under
# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
# ANY KIND, either express or implied. See the License for the specific language
# governing permissions and limitations under the License.

from .classes import BasePandasDataset

__all__ = ["BasePandasDataset"]
17 changes: 17 additions & 0 deletions modin/tests/config/docs_module_with_just_base/classes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Licensed to Modin Development Team under one or more contributor license agreements.
# See the NOTICE file distributed with this work for additional information regarding
# copyright ownership. The Modin Development Team licenses this file to you under the
# Apache License, Version 2.0 (the "License"); you may not use this file except in
# compliance with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software distributed under
# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
# ANY KIND, either express or implied. See the License for the specific language
# governing permissions and limitations under the License.


class BasePandasDataset:
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
15 changes: 13 additions & 2 deletions 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 @@ -507,10 +516,12 @@ def _inherit_docstrings_in_place(
obj,
overwrite_existing,
apilink,
parent_cls=cls_or_func,
parent_cls=base,
attr_name=attr,
)

_attributes_with_docstrings_replaced.add((base, attr))


def _inherit_docstrings(
parent: object,
Expand Down
Loading