Skip to content

BUG: #47350 if else added to add NaT for missing time values #47647

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

Closed
wants to merge 54 commits into from

Conversation

hamedgibago
Copy link

@hamedgibago hamedgibago commented Jul 8, 2022

If else added to add NaT rows for missing time values.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really sure what you are trying to fix. please add a relevant test that fails w/o a change.

@hamedgibago
Copy link
Author

not really sure what you are trying to fix. please add a relevant test that fails w/o a change.

Hi, I tried to fix bug #47350. Would you explain more about relevant test that fails w/o a change. I checked the code to return relevant results but some check failed during merge.

@hamedgibago hamedgibago requested a review from jreback July 9, 2022 07:21
@mroeschke
Copy link
Member

Please review https://pandas.pydata.org/pandas-docs/stable/development/contributing_codebase.html#contributing-to-the-code-base for instructions on how to submit a pull request so that the team understands the changes

@hamedgibago
Copy link
Author

Please review https://pandas.pydata.org/pandas-docs/stable/development/contributing_codebase.html#contributing-to-the-code-base for instructions on how to submit a pull request so that the team understands the changes

I checked most of the rules in the link. I don't know why most of the Ubuntu, windows and macos get error? Should I compile with any special parameters? Any idea @mroeschke @jreback

@mroeschke
Copy link
Member

From the logs, it appears your code change broke existing unit tests

https://github.com/pandas-dev/pandas/runs/7309922086?check_suite_focus=true

@mroeschke mroeschke added Datetime Datetime data dtype Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate labels Jul 22, 2022
@hamedgibago
Copy link
Author

@mroeschke @jreback 32 Bit Linux / pytest (pull_request) action raise error for test: FAILED pandas/tests/groupby/test_apply.py::test_group_apply_once_per_group[GH2936]. The error in 'test_group_apply_once_per_group' is that AttributeError: 'DataFrame' object has no attribute 'name'. What should I do with this test? I think dataframe.groupby name property belongs to old versions. This error in this test, prevents CI/CD tests to commit.

@mroeschke
Copy link
Member

Ideally all tests should still be passing even when the code fix is made.

@hamedgibago
Copy link
Author

@mroeschke Irrelevant codes reverted.

@hamedgibago hamedgibago requested a review from mroeschke August 7, 2022 14:34
@mroeschke
Copy link
Member

Appears you will need to merge in the main branch too.

@mroeschke
Copy link
Member

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.

@mroeschke mroeschke closed this Sep 12, 2022
@hamedgibago
Copy link
Author

Thank you, I would like to continue working on this bug. I was busy recently, but now really this bug is food for thought for me. I want to start working on it again.

@mroeschke mroeschke reopened this Sep 14, 2022
@hamedgibago
Copy link
Author

Thank you for reopening.

@mroeschke
Copy link
Member

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.

@mroeschke mroeschke closed this Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants