Skip to content

fix generate_range function in datetimes.py file #35348

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

fix generate_range function in datetimes.py file #35348

wants to merge 4 commits into from

Conversation

zky001
Copy link

@zky001 zky001 commented Jul 20, 2020 •
edited
Loading

change some issues be metioned at #35342

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@zky001
Update datetimes.py
66e9703 
change some issues be metioned at pandas-dev#35342
@zky001
Copy link
Author

zky001 commented Jul 20, 2020

testresult_20200720172043.png
I test all the cases on my machine but there still have some errors like pytz.exceptions.NonExistentTimeError: 1907-01-01 00:00:00
I want to know whether should I change NonExistentTimeError detect ways.

mroeschke
mroeschke requested changes Jul 20, 2020
View reviewed changes
Copy link
Member

@mroeschke mroeschke left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This would need a test.

Also I don't think this is an appropriate place for the fix. I imagine the bug is in generate_range

Could you give this PR a more informative title as well? Thanks.

@zky001 zky001 changed the title Update datetimes.py fix generate_range function in datetimes.py files Jul 23, 2020
@zky001 zky001 changed the title fix generate_range function in datetimes.py files fix generate_range function in datetimes.py file Jul 23, 2020
@zky001 zky001 requested a review from mroeschke July 23, 2020 06:01
@zky001
Copy link
Author

zky001 commented Jul 23, 2020 •
edited
Loading

This would need a test.

Also I don't think this is an appropriate place for the fix. I imagine the bug is in generate_range

Could you give this PR a more informative title as well? Thanks.

I have make some tests and they all passed,I will let PR have a more informative title。
image

zky001 added 3 commits July 23, 2020 14:19
@zky001
fix date_range returns wrong output in some cases
a1a4fe0 
change end time in generate_range function in datetimes file
@zky001
change comment before
28eff68 
fix issues be metioned at pandas-dev#35342
@zky001
delete comment
1de4293
mroeschke
mroeschke reviewed Jul 24, 2020
View reviewed changes
pandas/core/arrays/datetimes.py
@@ -2398,6 +2398,7 @@ def generate_range(start=None, end=None, periods=None, offset=BDay()):

elif end and not offset.is_on_offset(end):
end = offset.rollback(end)
end = Timestamp(str(end).split(' ')[0] + ' 23:59:59')
Copy link
Member

Choose a reason for hiding this comment

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

I think this fix is too specific. The bug probably lies in not checking the end bounds

@mroeschke
Copy link
Member

This PR still needs a test.

@simonjayhawkins simonjayhawkins mentioned this pull request Sep 15, 2020
Merged
@simonjayhawkins
Copy link
Member

@zky001 can you address #35348 (comment) and #35348 (review)

@dsaxton dsaxton added the Stale label Sep 15, 2020
@jbrockmendel
Copy link
Member

See my comment in #35342 on the appropriate fix

@dsaxton
Copy link
Member

dsaxton commented Sep 26, 2020

Closing as stale, @zky001 let us know if you'd still like to work on this

@dsaxton dsaxton closed this Sep 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke requested changes

Assignees
No one assigned
Labels
Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zky001 @mroeschke @simonjayhawkins @jbrockmendel @dsaxton