-
Notifications
You must be signed in to change notification settings - Fork 652
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-#5112: allows empty partition to be passed into query_compiler.dt_prop_map
#5133
Conversation
@@ -147,7 +147,10 @@ def _dt_prop_map(property_name): | |||
|
|||
def dt_op_builder(df, *args, **kwargs): | |||
"""Access specified date-time property of the passed frame.""" | |||
prop_val = getattr(df.squeeze(axis=1).dt, property_name) | |||
squeezed_df = df.squeeze(axis=1) | |||
if isinstance(squeezed_df, pandas.DataFrame) and len(squeezed_df.columns) == 0: |
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.
is it ever possible for squeezed_df
to ever be an empty Series?
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.
squeezed_df
can become an empty Series if df
is originally a DataFrame with a single column but no rows.
If squeezed_df
becomes an empty Series, there are 2 possible outcomes. If the Series dtype is datetime64[ns]
, it should return an empty DataFrame as expected. If the Series dtype is anything else, it will error because dt
is only accessible to datetimelike values.
Is there a situation where the passed in df
would be a DataFrame with a single column but no rows and the dtype is not datetime64[ns]
when it is supposed to be?
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.
I can't think of such a case.
c6d8e3f
to
791dd81
Compare
…y_compiler.dt_prop_map Signed-off-by: Bill Wang <billiam@ponder.io>
Signed-off-by: Bill Wang <billiam@ponder.io>
791dd81
to
3a82372
Compare
Signed-off-by: Bill Wang <billiam@ponder.io>
Signed-off-by: Bill Wang <billiam@ponder.io>
Signed-off-by: Bill Wang <billiam@ponder.io>
Signed-off-by: Bill Wang <billiam@ponder.io>
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.
I have some minor comments.
Signed-off-by: Bill Wang <billiam@ponder.io>
Signed-off-by: Bill Wang <billiam@ponder.io>
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
Might need to re-run CI here @Billy2551 . Just commit an empty commit to re-trigger the runs. |
Signed-off-by: Bill Wang billiam@ponder.io
What do these changes do?
Adds special case when empty partition is passed into
dt_prop_map
function. Previously, callingsqueeze
on a DataFrame partition with no columns would return the same DataFrame and then attempt to access a Series property. After the fix, a DataFrame partition with no columns simply returns an empty DataFrame.flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
git commit -s
docs/development/architecture.rst
is up-to-date