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

Bumping up min version for pyarrow #23482

Merged
merged 5 commits into from
Nov 5, 2018

Conversation

anjsudh
Copy link
Contributor

@anjsudh anjsudh commented Nov 4, 2018

closes #18742
closes #23409

  • tests passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@pep8speaks
Copy link

Hello @anjsudh! Thanks for submitting the PR.

@gfyoung gfyoung added IO Data IO issues that don't fit into a more specific label Dependencies Required and optional dependencies labels Nov 4, 2018
@gfyoung gfyoung requested review from jorisvandenbossche and jreback and removed request for jorisvandenbossche November 4, 2018 07:36
@jreback jreback added this to the 0.24.0 milestone Nov 4, 2018
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.

linting issues run: LINT=1 ci/code_checks.sh locally to see.

also can you put a check on fastparquet < 0.1 (or could do 0.1.2) i think is ok too

@jreback
Copy link
Contributor

jreback commented Nov 4, 2018

@martindurant what is the oldest fastparquet that we should target here?

@datapythonista
Copy link
Member

Can you run scripts/convert_deps.py, so ci/requirements-optional-pip.txt is updated.

@martindurant
Copy link
Contributor

Since this is for newer pandas, and fastparquet is pretty light on specific versions of its dependencies, I would keep with the most recent, 0.1.6, which is now 2.5 months old. I ought to put out a new version but wouldn't ask you to depend on it.

@jreback
Copy link
Contributor

jreback commented Nov 4, 2018

@martindurant well, this version of pandas could easily be installed on an existing setup, so easier to allow an older version unless we really need to upgrade it. ok let's go with 0.1.2 then.

@martindurant
Copy link
Contributor

0.1.2 is over a year old. OK, if the tests still pass...

My point was, if someone was wanting to upgrade their pandas, it should be reasonable for them to upgrade fastparquet too.

@jreback
Copy link
Contributor

jreback commented Nov 4, 2018

there is no easy way to force an upgrade as fp is just one of many optional dependencies

so much easier to have backward compatibility rather than annoy users

@codecov
Copy link

codecov bot commented Nov 5, 2018

Codecov Report

Merging #23482 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #23482      +/-   ##
==========================================
+ Coverage   92.23%   92.25%   +0.02%     
==========================================
  Files         161      161              
  Lines       51197    51176      -21     
==========================================
- Hits        47220    47214       -6     
+ Misses       3977     3962      -15
Flag Coverage Δ
#multiple 90.64% <100%> (+0.02%) ⬆️
#single 42.28% <14.28%> (+0.01%) ⬆️
Impacted Files Coverage Δ
pandas/io/parquet.py 83.5% <100%> (+9.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64c3491...91147aa. Read the comment docs.

@anjsudh
Copy link
Contributor Author

anjsudh commented Nov 5, 2018

so shall i go ahead with fastparquet 0.1.2 as minimum version?

Ran scripts/convert_deps.py to update dependencies in ci/requirements-optional-pip.txt
@anjsudh
Copy link
Contributor Author

anjsudh commented Nov 5, 2018

have set min version for fastparquet as 0.1.2.

@jreback
Copy link
Contributor

jreback commented Nov 5, 2018

you have a lint error

inting .py code
./pandas/io/parquet.py:8:1: F401 'pandas.Int64Index' imported but unused
./pandas/io/parquet.py:8:1: F401 'pandas.RangeIndex' imported but unused

ping on green.

@TomAugspurger TomAugspurger merged commit 5db3fae into pandas-dev:master Nov 5, 2018
@TomAugspurger
Copy link
Contributor

Thanks!

JustinZhengBC pushed a commit to JustinZhengBC/pandas that referenced this pull request Nov 14, 2018
tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Required and optional dependencies IO Data IO issues that don't fit into a more specific label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

we should bump our min of pyarrow to 0.7.0 COMPAT: drop pyarrow < 0.7 and fastparquet < 0.1 for Parquet IO
7 participants