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

Fix timezone metadata inference on parquet load #4655

Merged
merged 8 commits into from Apr 3, 2019

Conversation

Projects
None yet
3 participants
@martindurant
Copy link
Member

commented Mar 30, 2019

  • Tests added / passed
  • Passes flake8 dask

Will not pass until dask/fastparquet#411 is merged and released

Fixes #4640 (but not very elegantly!)

martindurant added some commits Mar 30, 2019

@martindurant

This comment has been minimized.

Copy link
Member Author

commented Mar 30, 2019

Does anyone know in which version of pyarrow timezones were added to the metadata?

martindurant added some commits Mar 30, 2019

Skip tz test for older fastparquet
Allows this to be merged before next release of fp
@martindurant

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

NB the codecov diff check does not seem to be measuring this PR particularly ( @jrbourbeau )

@pentschev

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

I believe this PR is fine from the codecov perspective. I'm still trying to understand what's going on with codecov, and I think it doesn't like PRs that don't have it's configuration file in it for some reason.

@martindurant would you mind merging current master in your branch so we can see if that solves codecov's complaints?

martindurant added some commits Apr 1, 2019

exclude coverage
in http - not related to this PR, but picked up by coveralls
@pentschev

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

It looks like Travis never returned its status here, it's been waiting for completion for about 12h now, but all tests have passed. Don't hold this due to that or codecov, go ahead and merge once reviewers are good ok with the changes.

@martindurant

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2019

Would someone like to review this? I'll probably merge this evening if no one speaks.

@martindurant martindurant merged commit 3205dbd into dask:master Apr 3, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@martindurant martindurant deleted the martindurant:parquet_tz branch Apr 3, 2019

@supeni86

This comment has been minimized.

Copy link

commented on 5d4a754 Apr 15, 2019

asmith26 added a commit to asmith26/dask that referenced this pull request Apr 22, 2019

Fix timezone metadata inference on parquet load (dask#4655)
* Fix timezone metadata inference on parquet load

* lint

* back compat

* compat correctly

* Skip tz test for older fastparquet

Allows this to be merged before next release of fp

* got it wrong...

* exclude coverage

in http - not related to this PR, but picked up by coveralls

jorge-pessoa pushed a commit to jorge-pessoa/dask that referenced this pull request May 14, 2019

Fix timezone metadata inference on parquet load (dask#4655)
* Fix timezone metadata inference on parquet load

* lint

* back compat

* compat correctly

* Skip tz test for older fastparquet

Allows this to be merged before next release of fp

* got it wrong...

* exclude coverage

in http - not related to this PR, but picked up by coveralls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.