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.

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

[#19431] Regression in make_block_same_class (tests failing for new fastparquet release) #19434

Merged
merged 23 commits into from
Jan 29, 2018
Merged

[#19431] Regression in make_block_same_class (tests failing for new fastparquet release) #19434

Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
eb55928
udpate test_parquet since fastparquet now handles tz
minggli Jan 28, 2018
2f4fc07
bring back dtype kwarg because it is needed for DatetimeTZBlock
minggli Jan 28, 2018
e18f8e6
version dependence test_datetime_tz
minggli Jan 28, 2018
9a07d97
separate test cases for new and old behaviour of fastparquet
minggli Jan 28, 2018
3d9810d
tidy test_datetime_tz to test old behaviour of fastparquet<0.14
minggli Jan 28, 2018
ee75fdf
rephase reason to skip test case for oldder fp
minggli Jan 28, 2018
68d6324
follow pytest fixture pattern as in pyarrow
minggli Jan 28, 2018
985081d
follow pyarrow test_basic style for fastparquet new behaviour>=0.1.4
minggli Jan 28, 2018
0cfcd37
fastparquet=0.1.3
minggli Jan 28, 2018
0c4a6d7
other api change
minggli Jan 28, 2018
6ce68cf
fix typo
minggli Jan 28, 2018
f414743
deprecation warning for dtype in make_block_same_class.
minggli Jan 28, 2018
0d76fe7
Future warning for dtype in make_block_same_class.
minggli Jan 28, 2018
bb95dc6
update notes as fastparquet nows supports timezone
minggli Jan 28, 2018
d9a2e2a
remove fastparquet pin
minggli Jan 29, 2018
97b17e9
remove other api change as it is internal
minggli Jan 29, 2018
800b741
remove version
minggli Jan 29, 2018
52220b9
remove dtype for make_block and DeprecationWarning on make_block_same…
minggli Jan 29, 2018
c602a76
FutureWarning on make_block_same_class
minggli Jan 29, 2018
ddbbde3
test case for dtype and warning generation
minggli Jan 29, 2018
6e6b5f0
issue number and simplify test
minggli Jan 29, 2018
326394f
misc doc
minggli Jan 29, 2018
77422ba
use pandas warning assert
minggli Jan 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
follow pyarrow test_basic style for fastparquet new behaviour>=0.1.4
  • Loading branch information
@minggli
minggli committed Jan 28, 2018
commit 985081d40e5897ae6c61d9d57352aea947db36c0
10 changes: 6 additions & 4 deletions pandas/tests/io/test_parquet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,13 +463,15 @@ def test_s3_roundtrip(self, df_compat, s3_resource, pa):

class TestParquetFastParquet(Base):

def test_basic(self, fp_ge_014, df_full):
def test_basic(self, fp, df_full):
Copy link
Contributor

Choose a reason for hiding this comment

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

make 2 tests here, one for < 0.1.4 and 1 for >

df = df_full

df['datetime_tz'] = pd.date_range('20130101', periods=3,
tz='US/Eastern')
# additional supported types for fastparquet>=0.1.4
Copy link
Member

Choose a reason for hiding this comment

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

you can leave out the ">=0.1.4" as the timedelta is not specific for the newer versions

if LooseVersion(pyarrow.__version__) >= LooseVersion('0.1.4'):
Copy link
Contributor

Choose a reason for hiding this comment

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

this is wrong, you mean fastparquet, yes? This is why the fixtures are FAR preferred

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Jeff, I am fine with either however, Joris suggested for now to keep it in line with pyarrow style as per #19434 (comment). I feel it's good to keep it consistent and follow up with another PR for styling for both PyArrow and FP.

Copy link
Member

Choose a reason for hiding this comment

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

@minggli only the 'pyarrow' -> 'fastparquet' on this line you need to change anyway (copy paste error)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose its ok. The problem is, this will be inconsistent with ALL the rest of pandas. You can make a simple change here to fix it. I would appreciate this done here, or ok in a followup . We spend an enormous amount of time making things consistent. adding inconsistent code is not great, even for expediency.

df['datetime_tz'] = pd.date_range('20130101', periods=3,
tz='US/Eastern')
df['timedelta'] = pd.timedelta_range('1 day', periods=3)
check_round_trip(df, fp_ge_014)
check_round_trip(df, fp)

@pytest.mark.skip(reason="not supported")
def test_duplicate_columns(self, fp):
Expand Down