Skip to content

Commit

Permalink
FIX-#7113: Fix docstring overrides for subclasses. (#7354)
Browse files Browse the repository at this point in the history
Signed-off-by: sfc-gh-mvashishtha <mahesh.vashishtha@snowflake.com>
  • Loading branch information
sfc-gh-mvashishtha authored Aug 1, 2024
1 parent 24018db commit 22ed4d8
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 2 deletions.
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():
"""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():
"""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

0 comments on commit 22ed4d8

Please sign in to comment.