-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Updated condition to skip for pytables build issue on numpy 1.15 #22098 #22522
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fetch and merge master? That should fix the circleCI failure.
pandas/compat/numpy/__init__.py
Outdated
@@ -74,5 +75,6 @@ def np_array_datetime64_compat(arr, *args, **kwargs): | |||
'_np_version_under1p12', | |||
'_np_version_under1p13', | |||
'_np_version_under1p14', | |||
'_np_version_under1p15' | |||
'_np_version_under1p15', | |||
'_np_version_equal1p15' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nint: add a trailing comma
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comma required after '_np_version_equal1p15'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but it makes the next diff nicer.
pandas/tests/io/test_pytables.py
Outdated
@@ -2192,7 +2192,7 @@ def test_unimplemented_dtypes_table_columns(self): | |||
pytest.raises(TypeError, store.append, 'df_unimplemented', df) | |||
|
|||
@pytest.mark.skipif( | |||
not _np_version_under1p15, | |||
_np_version_equal1p15, | |||
reason=("pytables conda build package needs build " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could update this message to link to the upstream numpy issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
I have fetched and merged master. But still shows a build error. Anything wrong with my code. |
Looks like numpy 1.15 was installed. Anaconda doesn't seem to have 1.15.1 built yet https://repo.continuum.io/pkgs/main/linux-64/. Will probably be up this week. |
pandas/compat/numpy/__init__.py
Outdated
@@ -15,6 +15,7 @@ | |||
_np_version_under1p13 = _nlv < LooseVersion('1.13') | |||
_np_version_under1p14 = _nlv < LooseVersion('1.14') | |||
_np_version_under1p15 = _nlv < LooseVersion('1.15') | |||
_np_version_equal1p15 = _nlv == LooseVersion('1.15') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the pattern to change this to
_np_version_under1p16 = _nlv < LooseVersion('1.16')
_np_version_equal1p16 = _nlv == LooseVersion('1.16')
when numpy 1.16 comes out? Or is this a one off to meet a current need?
This is for 1.15.0 exactly.
It'd probably be best to define a skipif for NumPy 1.15.0 in
https://github.com/pandas-dev/pandas/blob/master/pandas/util/_test_decorators.py
and just use that in the test.
…On Mon, Aug 27, 2018 at 1:47 PM Jeff ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pandas/compat/numpy/__init__.py
<#22522 (comment)>:
> @@ -15,6 +15,7 @@
_np_version_under1p13 = _nlv < LooseVersion('1.13')
_np_version_under1p14 = _nlv < LooseVersion('1.14')
_np_version_under1p15 = _nlv < LooseVersion('1.15')
+_np_version_equal1p15 = _nlv == LooseVersion('1.15')
Is the pattern to change this to
_np_version_under1p16 = _nlv < LooseVersion('1.16')
_np_version_equal1p16 = _nlv == LooseVersion('1.16')
when numpy 1.16 comes out? Or is this a one off to meet a current need?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#22522 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQHIu-1HRG3_ocxKBbOoJciz0eyZQAmks5uVD7DgaJpZM4WN2cT>
.
|
@@ -2192,9 +2192,9 @@ def test_unimplemented_dtypes_table_columns(self): | |||
pytest.raises(TypeError, store.append, 'df_unimplemented', df) | |||
|
|||
@pytest.mark.skipif( | |||
not _np_version_under1p15, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't define anything new here, juse use LooseVersion explicity to check for 1.15.0 as this is the only version affected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed it. Could you please have a look at it @jreback. let me know if changes required.
pandas/tests/io/test_pytables.py
Outdated
not _np_version_under1p15, | ||
reason=("pytables conda build package needs build " | ||
"with numpy 1.15: gh-22098")) | ||
LooseVersion(np.__version__) == LooseVersion('1.15'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need 1.15.0 here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use StrictVersion here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I see LooseVersion('1.15') and LooseVersion('1.15.0') are not equal. But, StrictVersion('1.15') and Strict Version('1.15.0') are equal. We can use StrictVersion, in that we can use '1.15' or '1.15.0'. If we are using LooseVersion, then it should be '1.15.0'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it safe to use StrictVersion then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made changes with LooseVersion('1.15.0'). I think this will suffice. Please let me know if we have to use StrictVersion instead. @jreback @TomAugspurger
thanks! |
git diff upstream/master -u -- "*.py" | flake8 --diff