FIX-#7248: Make sure _validate_dtypes_sum_prod_mean works correctly with datetime types
#7237
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rks correctly with datetime types Signed-off-by: Anatoly Myachev <[email protected]>
anmyachev
changed the title
_validate_dtypes_sum_prod_mean_validate_dtypes_sum_prod_mean works correctly with datetime types
anmyachev
requested review from
devin-petersohn,
mvashishtha,
RehanSD,
YarShev,
vnlitvinov,
dchigarev and
a team
as code owners
anmyachev
commented
May 8, 2024
Comment on lines
+2158
to
+2169
| # We cannot add datetime types, so if we are summing a column with | ||
| # dtype datetime64 and cannot ignore non-numeric types, we must throw a | ||
| # TypeError. | ||
| if numeric_only is False and any( | ||
| dtype == pandas.api.types.pandas_dtype("datetime64[ns]") | ||
| for dtype in self.dtypes | ||
| ): | ||
| raise TypeError( | ||
| "'DatetimeArray' with dtype datetime64[ns] does not support reduction 'sum'" | ||
| ) | ||
|
|
||
| data = self._get_numeric_data(axis) if numeric_only else self |
There was a problem hiding this comment.
This piece of code has been moved from _validate_dtypes_sum_prod_mean function because the processing is different for sum function.
anmyachev
commented
May 8, 2024
Comment on lines
-3064
to
-3065
| dtype != pandas.api.types.pandas_dtype("datetime64[ns]") | ||
| and dtype != pandas.api.types.pandas_dtype("timedelta64[ns]") |
There was a problem hiding this comment.
Now these types cannot be compared with numbers.
anmyachev
commented
May 8, 2024
| # numeric_only is False because if it is None, it will default to True | ||
| # if the operation fails with mixed dtypes. | ||
| if ( | ||
| axis | ||
| and numeric_only is False | ||
| and np.unique([is_numeric_dtype(dtype) for dtype in self.dtypes]).size == 2 | ||
| and not all([is_numeric_dtype(dtype) for dtype in self.dtypes]) |
There was a problem hiding this comment.
The new way is more obvious.
YarShev
approved these changes
May 12, 2024
YarShev
reviewed
May 12, 2024
| modin_df, | ||
| pandas_df, | ||
| lambda df: df.min(axis=1), | ||
| # pandas raises: `TypeError: '<=' not supported between instances of 'Timestamp' and 'int'` |
There was a problem hiding this comment.
What prevents us from emitting the same error message?
There was a problem hiding this comment.
Most likely there are no fundamental restrictions, but we need to know better how pandas does this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What do these changes do?
flake8 modin/ asv_bench/benchmarks scripts/doc_checker.pyblack --check modin/ asv_bench/benchmarks scripts/doc_checker.pygit commit -s_validate_dtypes_sum_prod_meandoesn't process correctlydatetime64[ns]andtimedelta64[ns]types #7248docs/development/architecture.rstis up-to-date