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

Updates for fastparquet evolution #9650

Merged
merged 8 commits into from Nov 16, 2022
Merged

Conversation

martindurant
Copy link
Member

@martindurant martindurant commented Nov 11, 2022

  • Closes #xxxx
  • Tests added / passed
  • Passes pre-commit run --all-files

Makes dask/fastparquet#818 pass

Copy link
Contributor

@phobson phobson left a comment

Choose a reason for hiding this comment

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

Thanks, @martindurant

@martindurant
Copy link
Member Author

@rjzamora : a quick and simple one, if you have a moment

Copy link
Member

@rjzamora rjzamora left a comment

Choose a reason for hiding this comment

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

One minor comment fix. Otherwise, LGTM

dask/dataframe/io/parquet/fastparquet.py Outdated Show resolved Hide resolved
Co-authored-by: Richard (Rick) Zamora <rzamora217@gmail.com>
Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @martindurant. I think we want to trigger an upstream CI build by including a commit with test-upstream in it so we can make sure fastparquet_version > parse_version("0.8.3") is True (note you'll also want to merge main as there were some changes to fix the upstream build that were just merged)

@martindurant
Copy link
Member Author

There is no release of fastparquet yet, since dask/fastparquet#818 is waiting on this PR

@jrbourbeau
Copy link
Member

The upstream build will install fastparquet directly from main. I assumed this was using some new feature that's not yet released, but in fastparquets main branch -- is that not the case?

@martindurant
Copy link
Member Author

No, I didn't even merge it; but I will now, just to ease the impasse :).
fastparquet also tests against main, of course!

@martindurant
Copy link
Member Author

done

@martindurant
Copy link
Member Author

dask/array/tests/test_stats.py::test_two[ttest_1samp-kwargs2] is nothing to do with this PR

@jrbourbeau jrbourbeau changed the title Updates for fastparquet evolution Updates for fastparquet evolution Nov 16, 2022
@jrbourbeau jrbourbeau merged commit 6a02bcb into dask:main Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants