Skip to content

arrays/period: allow parsing of PeriodDtype columns from read_csv #27380

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 3 commits into from
Jul 17, 2019
Merged

arrays/period: allow parsing of PeriodDtype columns from read_csv #27380

merged 3 commits into from
Jul 17, 2019

Conversation

chibby0ne
Copy link
Contributor

@chibby0ne chibby0ne commented Jul 13, 2019 •
edited
Loading

Fixes: #26934

Signed-off-by: Antonio Gutierrez [email protected]

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

@pep8speaks
Copy link

pep8speaks commented Jul 13, 2019 •
edited
Loading

Hello @chibby0ne! 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 2019-07-17 01:10:07 UTC

jreback
jreback requested changes Jul 13, 2019
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.

tests

@chibby0ne chibby0ne force-pushed the parse_periodtype_columns_read_csv branch 2 times, most recently from bcfaeed to 457288d Compare July 13, 2019 22:20
@chibby0ne
Copy link
Contributor Author

tests

Done! Thank you for the swift reply ;)!

@chibby0ne
Copy link
Contributor Author

Can someone explain how the codecov/project failed? It looks as if the coverage in all the files dropped, even though I only changed 2 files.

@jbrockmendel
Copy link
Member

Can someone explain how the codecov/project failed?

Happens sometimes, seems random. Rebase and push again and it usually fixes itself

@chibby0ne chibby0ne force-pushed the parse_periodtype_columns_read_csv branch from 457288d to ba5f438 Compare July 14, 2019 16:52
@chibby0ne
Copy link
Contributor Author

Can someone explain how the codecov/project failed?

Happens sometimes, seems random. Rebase and push again and it usually fixes itself

Weird. Thanks for the suggestion though ;)

@WillAyd WillAyd added Period Period data type IO CSV read_csv, to_csv labels Jul 15, 2019
@WillAyd WillAyd added this to the 0.25.0 milestone Jul 15, 2019
WillAyd
WillAyd approved these changes Jul 15, 2019
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

lgtm

TomAugspurger
TomAugspurger reviewed Jul 15, 2019
View reviewed changes
doc/source/whatsnew/v0.25.0.rst Outdated
@@ -1186,6 +1186,7 @@ ExtensionArray
- :meth:`Series.count` miscounts NA values in ExtensionArrays (:issue:`26835`)
- Added ``Series.__array_ufunc__`` to better handle NumPy ufuncs applied to Series backed by extension arrays (:issue:`23293`).
- Keyword argument ``deep`` has been removed from :meth:`ExtensionArray.copy` (:issue:`27083`)
- Allow parsing of `PeriodDType` columns when using `read_csv()` (:issue:`26934`)
Copy link
Contributor

@TomAugspurger TomAugspurger Jul 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
- Allow parsing of `PeriodDType` columns when using `read_csv()` (:issue:`26934`)
- Allow parsing of :class:`PeriodDtype` columns when using `read_csv()` (:issue:`26934`)

jreback
jreback requested changes Jul 15, 2019
View reviewed changes
doc/source/whatsnew/v0.25.0.rst Outdated
@@ -1186,6 +1186,7 @@ ExtensionArray
- :meth:`Series.count` miscounts NA values in ExtensionArrays (:issue:`26835`)
- Added ``Series.__array_ufunc__`` to better handle NumPy ufuncs applied to Series backed by extension arrays (:issue:`23293`).
- Keyword argument ``deep`` has been removed from :meth:`ExtensionArray.copy` (:issue:`27083`)
- Allow parsing of :class:`PeriodDtype` columns when using `read_csv()` (:issue:`26934`)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be in the IO section

use :func:`read_csv`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done thanks a lot for the quick feedback again!

@chibby0ne chibby0ne force-pushed the parse_periodtype_columns_read_csv branch from 48d746b to 074fdbc Compare July 16, 2019 05:54
@chibby0ne chibby0ne force-pushed the parse_periodtype_columns_read_csv branch from 074fdbc to cca5c49 Compare July 17, 2019 01:10
@jreback jreback merged commit 89d79c5 into pandas-dev:master Jul 17, 2019
@jreback
Copy link
Contributor

jreback commented Jul 17, 2019

thanks @chibby0ne

@chibby0ne chibby0ne deleted the parse_periodtype_columns_read_csv branch July 17, 2019 13:53
@chibby0ne
Copy link
Contributor Author

thanks @chibby0ne

Thank you for your support @jreback @TomAugspurger @jbrockmendel @WillAyd !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbrockmendel jbrockmendel jbrockmendel left review comments

@WillAyd WillAyd WillAyd approved these changes

@jreback jreback jreback approved these changes

@TomAugspurger TomAugspurger TomAugspurger approved these changes

@gfyoung gfyoung gfyoung approved these changes

Assignees
No one assigned
Labels
IO CSV read_csv, to_csv Period Period data type
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.

Issue with parsing PeriodDtype columns in read_csv()
7 participants
@chibby0ne @pep8speaks @jbrockmendel @jreback @WillAyd @TomAugspurger @gfyoung