-
Notifications
You must be signed in to change notification settings - Fork 670
REFACTOR-#6805: Move all IO functions to 'modin.pandas.io' module #6806
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
Conversation
…' module Signed-off-by: Dmitry Chigarev <[email protected]>
| from modin.error_message import ErrorMessage | ||
| from modin.logging import disable_logging | ||
| from modin.pandas import Categorical | ||
| from modin.pandas.io import from_non_pandas, from_pandas, to_pandas |
Check notice
Code scanning / CodeQL
Cyclic import
| def DataFrame(cls): | ||
| """Get ``modin.pandas.DataFrame`` class.""" | ||
| if cls._dataframe is None: | ||
| from .dataframe import DataFrame |
Check notice
Code scanning / CodeQL
Cyclic import
| from modin.config import PersistentPickle | ||
| from modin.logging import disable_logging | ||
| from modin.utils import MODIN_UNNAMED_SERIES_LABEL, _inherit_docstrings, to_pandas | ||
| from modin.pandas.io import from_pandas, to_pandas |
Check notice
Code scanning / CodeQL
Cyclic import
| from .iterator import PartitionIterator | ||
| from .series_utils import CategoryMethods, DatetimeProperties, StringMethods | ||
| from .utils import _doc_binary_op, cast_function_modin2pandas, from_pandas, is_scalar | ||
| from .utils import _doc_binary_op, cast_function_modin2pandas, is_scalar |
Check notice
Code scanning / CodeQL
Cyclic import
Signed-off-by: Dmitry Chigarev <[email protected]>
Signed-off-by: Dmitry Chigarev <[email protected]>
Signed-off-by: Dmitry Chigarev <[email protected]>
| """ | ||
| return modin_obj._to_pandas() | ||
|
|
||
| def deprecated_func(*args: tuple[Any], **kwargs: dict[Any, Any]) -> Any: |
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.
Should we use @functools.wraps(func) here or assign some attributes like name, doc to deprecated_func from a deprecated function to be called?
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.
We can't do this. The code building a decorator is being executed at the time of the module's initialization, which means, that we cannot do circular imports at this point. See the simplified example:
def create_deprecated(msg):
# the code in this function is being executed at the time of module initialization,
# meaning that the following line will trigger the circular import error:
# from modin.pandas.io import to_pandas
# can't use this as we haven't imported 'to_pandas' yet
# @functols.wrap(to_pandas)
def wrapper(*args, **kwargs):
# we can only import actual 'to_pandas' here as this code is being executed
# when the function is called and all the modules are initialized
from modin.pandas.io import to_pandas
return to_pandas(*args, **kwargs)
return wrapper
to_pandas = create_deprecated("to_pandas")|
Left a few comments. Other than that, LGTM. |
modin/pandas/io.py
Outdated
| @classmethod | ||
| @property |
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.
Let's not use them together: https://stackoverflow.com/questions/128573/using-property-on-classmethods/64738850#64738850
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.
changed to using this as a classmethod-constructor
it didn't work, will find another solution
UPD: ended up writing a custom-written decorator allowing class properties
Signed-off-by: Dmitry Chigarev <[email protected]>
Signed-off-by: Dmitry Chigarev <[email protected]>
Signed-off-by: Dmitry Chigarev <[email protected]>
anmyachev
left a comment
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.
LGTM!
What do these changes do?
This PR moves the following IO function to the
modin.pandas.iomodule:from_pandas()from_arrow()from_dataframe()from_non_pandas()to_pandas()to_numpy()The
try_cast_to_pandas()function was not moved as IMO it's more of a utility function rather than a pure IO. However, if reviewers think differently, I still can move it to the IO module.flake8 modin/ asv_bench/benchmarks scripts/doc_checker.pyblack --check modin/ asv_bench/benchmarks scripts/doc_checker.pygit commit -smodin.pandas.iomodule #6805docs/development/architecture.rstis up-to-date