From 9890eaec3a834a8fde9839fa3970f65129ae63c3 Mon Sep 17 00:00:00 2001 From: sfc-gh-mvashishtha Date: Wed, 31 Jul 2024 10:53:19 -0700 Subject: [PATCH 1/2] FIX-#7113: Fix docstring overrides for subclasses. Signed-off-by: sfc-gh-mvashishtha --- modin/tests/config/docs_module/classes.py | 3 +++ modin/tests/config/test_envvars.py | 18 ++++++++++++++++++ modin/utils.py | 13 ++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/modin/tests/config/docs_module/classes.py b/modin/tests/config/docs_module/classes.py index 235c99bdf0f..9a8eabac61b 100644 --- a/modin/tests/config/docs_module/classes.py +++ b/modin/tests/config/docs_module/classes.py @@ -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.""" diff --git a/modin/tests/config/test_envvars.py b/modin/tests/config/test_envvars.py index d057ecb0299..17ac0b58786 100644 --- a/modin/tests/config/test_envvars.py +++ b/modin/tests/config/test_envvars.py @@ -12,6 +12,7 @@ # governing permissions and limitations under the License. import os +import sys import pandas import pytest @@ -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." @@ -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(): diff --git a/modin/utils.py b/modin/utils.py index 8623732671b..237fffa96b6 100644 --- a/modin/utils.py +++ b/modin/utils.py @@ -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: """ @@ -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] @@ -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 @@ -511,6 +520,8 @@ def _inherit_docstrings_in_place( attr_name=attr, ) + _attributes_with_docstrings_replaced.add((base, attr)) + def _inherit_docstrings( parent: object, From 45e76696eeba38352cfe2df0968ac5a1a8f03572 Mon Sep 17 00:00:00 2001 From: sfc-gh-mvashishtha Date: Wed, 31 Jul 2024 14:55:47 -0700 Subject: [PATCH 2/2] Try fixing tests Signed-off-by: sfc-gh-mvashishtha --- .../docs_module_with_just_base/__init__.py | 16 ++++++++++++++++ .../docs_module_with_just_base/classes.py | 17 +++++++++++++++++ modin/utils.py | 2 +- 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 modin/tests/config/docs_module_with_just_base/__init__.py create mode 100644 modin/tests/config/docs_module_with_just_base/classes.py diff --git a/modin/tests/config/docs_module_with_just_base/__init__.py b/modin/tests/config/docs_module_with_just_base/__init__.py new file mode 100644 index 00000000000..f2da948e26c --- /dev/null +++ b/modin/tests/config/docs_module_with_just_base/__init__.py @@ -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"] diff --git a/modin/tests/config/docs_module_with_just_base/classes.py b/modin/tests/config/docs_module_with_just_base/classes.py new file mode 100644 index 00000000000..645c7c63df6 --- /dev/null +++ b/modin/tests/config/docs_module_with_just_base/classes.py @@ -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.""" diff --git a/modin/utils.py b/modin/utils.py index 237fffa96b6..6c17b1b12d3 100644 --- a/modin/utils.py +++ b/modin/utils.py @@ -516,7 +516,7 @@ def _inherit_docstrings_in_place( obj, overwrite_existing, apilink, - parent_cls=cls_or_func, + parent_cls=base, attr_name=attr, )