Skip to content
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

ARROW-622 [Python] deprecate timestamps_to_ms in .from_pandas() #1046

Closed
wants to merge 1 commit into from

Conversation

jreback
Copy link
Contributor

@jreback jreback commented Sep 5, 2017

xref pandas-dev/pandas#17438

this was not fully resolved in #944

@@ -701,18 +701,25 @@ cdef class Table:
return result

@classmethod
def from_pandas(cls, df, bint timestamps_to_ms=False,
def from_pandas(cls, df,
object coerce_timestamps=False, object timestamps_to_ms=None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that the signature is slightly changed here; I could add the new argument at the end if you prefer.

@jreback
Copy link
Contributor Author

jreback commented Sep 5, 2017

green other that a stalled build: https://travis-ci.org/apache/arrow/jobs/272034734

Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

I'm -1 on this change unless there is definitely no other way. Can this be handled by passing coerce_timestamps to parquet.write_table?

Convert datetime columns to the indicated resolution. If False
preserve the existing dtype.
This is needed for compability with other functionality like
Parquet I/O which only supports milliseconds / microseconds.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is the right fix. The coercion happens in parquet.write_table

@jreback
Copy link
Contributor Author

jreback commented Sep 6, 2017

ok this is more limited in scope, removing test warnings for using timestamp_to_ms in .from_pandas().

I'll fix the deprecation issue in .to_parquet on the pandas side itself.

@wesm
Copy link
Member

wesm commented Sep 6, 2017

LGTM. The build is failing due to a Sphinx warning, can you fix that?

@wesm
Copy link
Member

wesm commented Sep 7, 2017

+1. Build failed due to stalled macos image and maven central flakiness

@asfgit asfgit closed this in b698227 Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants