Skip to content

BUG : Series.to_timestamp and Series.to_period raise user-facing AssertionError #34067

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
Merged
merged 10 commits into from
May 20, 2020
Merged

BUG : Series.to_timestamp and Series.to_period raise user-facing AssertionError #34067

merged 10 commits into from
May 20, 2020

Conversation

scriptdruid
Copy link
Contributor

@scriptdruid scriptdruid commented May 8, 2020 •
edited
Loading

@scriptdruid scriptdruid changed the title resolves series.py issue #33327 FIX : Resolves to_timestamp and to_period in series.py issue #33327 May 8, 2020
@simonjayhawkins
Copy link
Member

Thanks @vipulrai91 for the PR. can you add tests that test for the error message.

@simonjayhawkins simonjayhawkins added Error Reporting Incorrect or improved errors from pandas Period Period data type Datetime Datetime data dtype labels May 8, 2020
@TomAugspurger
Copy link
Contributor

Also a whatsnew in doc/source/whatsnew/v1.1.0rst

@scriptdruid
Copy link
Contributor Author

Also a whatsnew in doc/source/whatsnew/v1.1.0rst

Yes , I would add it once the above part is fixed.

@pep8speaks
Copy link

pep8speaks commented May 12, 2020 •
edited
Loading

Hello @vipulrai91! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-05-20 11:42:15 UTC

@scriptdruid scriptdruid force-pushed the AssertionErrorSeriesTimestamp branch from bbc5a4d to 680318a Compare May 12, 2020 11:01
@simonjayhawkins
Copy link
Member

Should the test be like this

we use with pytest.raises(..., match=...) pattern

pandas/tests/series/test_period.py

pandas\tests\series\methods\test_to_period.py and pandas\tests\series\methods\test_to_timestamp.py

Also a whatsnew in doc/source/whatsnew/v1.1.0rst

Yes , I would add it once the above part is fixed.

add in Backwards incompatible API changes section

As this previously raised AttributeError, #33327 (comment), can add similar to existing release note for swaplevels

- :meth:`DataFrame.swaplevels` now raises a  ``TypeError`` if the axis is not a :class:`MultiIndex`.
  Previously an ``AttributeError`` was raised (:issue:`31126`)

@simonjayhawkins simonjayhawkins added this to the 1.1 milestone May 12, 2020
jreback
jreback requested changes May 12, 2020
View reviewed changes
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.

can you add a test that hits this

@scriptdruid
Copy link
Contributor Author

Thanks @vipulrai91 for the PR. can you add tests that test for the error message.

can you add a test that hits this

@jreback
added tests at : pandas/tests/series/methods/test_to_period.py and
pandas/tests/series/methods/test_to_timestamp.py

Please let me know if this is correct , also , I am not sure which parts to add the doc in doc/source/whatsnew/v1.1.0.rst

@scriptdruid scriptdruid requested a review from jreback May 12, 2020 13:32
@simonjayhawkins
Copy link
Member

I am not sure which parts to add the doc in doc/source/whatsnew/v1.1.0.rst

see #34067 (comment)

simonjayhawkins
simonjayhawkins requested changes May 20, 2020
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @vipulrai91 for working on this. a few more suggestions but getting close.

@simonjayhawkins simonjayhawkins changed the title FIX : Resolves to_timestamp and to_period in series.py issue #33327 BUG : Series.to_timestamp and Series.to_period raise AssertionError May 20, 2020
@simonjayhawkins simonjayhawkins changed the title BUG : Series.to_timestamp and Series.to_period raise AssertionError BUG : Series.to_timestamp and Series.to_period raise user-facing AssertionError May 20, 2020
simonjayhawkins
simonjayhawkins approved these changes May 20, 2020
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @vipulrai91 lgtm pending green ex. whitespace in whatsnew.

simonjayhawkins
simonjayhawkins approved these changes May 20, 2020
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @vipulrai91 lgtm. @jreback

@jreback jreback merged commit d3a6a3a into pandas-dev:master May 20, 2020
@jreback
Copy link
Contributor

jreback commented May 20, 2020

thanks @vipulrai91 very nice

PuneethaPai pushed a commit to PuneethaPai/pandas that referenced this pull request May 22, 2020
@scriptdruid @PuneethaPai
BUG : Series.to_timestamp and Series.to_period raise user-facing Asse…
ad232db 
…rtionError (pandas-dev#34067)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

@simonjayhawkins simonjayhawkins simonjayhawkins approved these changes

Assignees
No one assigned
Labels
Datetime Datetime data dtype Error Reporting Incorrect or improved errors from pandas Period Period data type
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.

BUG: User-facing AssertionError in Series.to_timestamp
5 participants
@scriptdruid @simonjayhawkins @TomAugspurger @pep8speaks @jreback