Numexpr 2.4.6 #15383
Numexpr 2.4.6 #15383
Conversation
| @FrancescAlted we have a few occurences of numpexpr pinned at older versions in ci/requirement files (I think the 2.7_COMPAT.run and 3.4_SLOW.run ones). Best to change those versions to 2.4.6 then. |
doc/source/install.rst Outdated
| * `numexpr <https://github.com/pydata/numexpr>`__: for accelerating certain numerical operations. | ||
| ``numexpr`` uses multiple cores as well as smart chunking and caching to achieve large speedups. | ||
| If installed, must be Version 2.1 or higher (excluding a buggy 2.4.4). Version 2.4.6 or higher is highly recommended. | ||
| If installed, must be Version 2.4.6 or higher. Numexpr 2.6.2 or higher is strongly recommended. |
There was a problem hiding this comment.
you don't need the recommended part
There was a problem hiding this comment.
Well, IMO the improvements in https://github.com/pydata/numexpr/blob/master/RELEASE_NOTES.rst#changes-from-261-to-262 are worth a recommendation, but I can remove it if you insist.
There was a problem hiding this comment.
it may be but back compat is more important
doc/source/whatsnew/v0.20.0.txt Outdated
| | ||
| .. warning:: | ||
| | ||
| Due to important fixes and performance improvements, ``numexpr`` version is now required to be >= 2.4.6 and it will not be used at all if this requisite is not fullfilled (:issue:`15213`). |
There was a problem hiding this comment.
i would just move this to the other api changes section; just say 2.4.6 is now required
There was a problem hiding this comment.
You probably mean "Other enhancements" section (numexpr does not introduce any API change).
There was a problem hiding this comment.
it's an API change for pandas meaning if u has a version that is no longer supported then things will change - its also something to be aware of when upgrading
| # has some hard-to-diagnose bugs | ||
| if ver == LooseVersion('2.4.4'): | ||
| _NUMEXPR_INSTALLED = False | ||
| warnings.warn( |
Codecov Report
@@ Coverage Diff @@ ## master #15383 +/- ## ========================================== - Coverage 90.42% 90.41% -0.02% ========================================== Files 134 134 Lines 49357 49375 +18 ========================================== + Hits 44632 44640 +8 - Misses 4725 4735 +10
Continue to review full report at Codecov.
|
| There are a couple of "allowed failures" in TravisCI. Any hint on what's going on there? |
| For the MacOSX one, I see |
| Uh, I see the Mac OSX build that completed just fine. Also, the AppVeyor failure seems like a transitory failure during conda packages access: But I am still unsure on the level of importance of the "allowed failures" in TravisCI. |
| Sorry, I mislooked, not the MacOSX build of course, build number 6 with the old versions (2.7_COMPAT), the error now is: We want to keep the numpy 1.7 there for now (although there is an issue to bump that requirement as well), so I would just keep the older numexpr in this build (then it is a test that you get the correct error for that).
They are not 'allowed' to fail in principle (only for specific things, eg compat issue with numpy master that is not yet fixed). I think it is like that to get quicker feedback from travis. There is still a lint error: The appveyor issue indeed does not seem to be related to this PR. |
| @FrancescAlted Was it needed to also revert the version change in the 3.4_SLOW build? As this one was not failing I think. |
| Yes, it was: https://travis-ci.org/pandas-dev/pandas/builds/201238220. I am not familiar with the convention to run different configurations, but I'd say that stems from: https://github.com/pandas-dev/pandas/blob/master/.travis.yml#L131. I just reverted the numexpr versions existing in this two scenarios. My intention is to look at the possible errors after the revert and try to fix them. |
| i would change this version to 2.4.6, otherwise lgtm. https://github.com/pandas-dev/pandas/blob/master/ci/requirements-3.4_SLOW.run ping on green. |
| I think we will still have a remaining error here. But the assert will not raise the error as such, and we would need something like |
| So probably that test never worked .. What you could do is move the import inside the test and in a "catch warnings" context, and then check that a warning is raised in the specific case and not otherwise (https://docs.python.org/3.5/library/warnings.html#testing-warnings)? |
| no it worked but the import process is changed by this PR |
| I have been looking at the original code, and I don't think it worked because |
| so I think with the changed up order of tests, this So I guess just change the test then. |
| Ok, I think that I have found the issue and fixed it in e1b34a9. Basically, one needs to force the |
| Well, it turns out that |
| @FrancescAlted I would just remove the check for the warning (leave the test just don't assert that it produces) |
| from pandas.computation.engines import _engines | ||
| import pandas.computation.expr as expr | ||
| from pandas.computation import _MIN_NUMEXPR_VERSION | ||
| # Get reload for Python 3.4 and on, if not, use internal reload() |
There was a problem hiding this comment.
too complicated, don't reload
| assert not _NUMEXPR_INSTALLED | ||
| elif ver < LooseVersion('2.1'): | ||
| if ver < LooseVersion(_MIN_NUMEXPR_VERSION): | ||
| with tm.assert_produces_warning(UserWarning, |
There was a problem hiding this comment.
just remove this assert_produces_warning
| with tm.assert_produces_warning(UserWarning, | ||
| check_stacklevel=False): | ||
| assert not _NUMEXPR_INSTALLED | ||
| reload(pd.computation) |
| lgtm. ping on green. |
| Done. Hopefully this will give us all green (bar some temporary failures that apparently are usual in TravisCI). |
| @FrancescAlted well there have been some flakiness with some gbq tests; I turned them off for now. We should have all green. FYI we just have the fail fast to make the results appear quicker. We have LOTS of configurations to test. So allowed failures are really NOT allowed :> |
| one more FYI. every push on a PR generates a new travis run (which can take hours). So generally like to bunch them up if possible. This PR of course is a case that its necessary to test little changes so no big deal. |
| thanks @FrancescAlted |
closes pandas-dev#15213 Author: Francesc Alted <francesc@continuum.io> Closes pandas-dev#15383 from FrancescAlted/numexpr-2.4.6 and squashes the following commits: c417fe2 [Francesc Alted] Simplify and remove UserWarning testing on numexpr import e1b34a9 [Francesc Alted] Force a reload of pd.computation for actually triggering the UserWarning c081199 [Francesc Alted] Relax the exact message for the ImportError 73f0319 [Francesc Alted] numexpr requisite raised to 2.4.6 0d4ab9a [Francesc Alted] Restored the old numexpr version dependencies to adjust for old requirements c1aae19 [Francesc Alted] Fixed a lint error 7575ba2 [Francesc Alted] Using constants instead of literals for numexpr version 7a275ce [Francesc Alted] Fixed a typo 93f54aa [Francesc Alted] numexpr section moved to Other API changes section 3b6e58b [Francesc Alted] Removed recomendation for numexpr 2.6.2 f225598 [Francesc Alted] Updated test_compat for numexpr 2.4.6 8bd4ed1 [Francesc Alted] numexpr 2.4.6 requirement moved to other enhancements section e45b742 [Francesc Alted] Moved pinned versions in CI folder to 2.4.6 6e12e29 [Francesc Alted] Added a notice on the recommended numexpr version ac62653 [Francesc Alted] Require numexpr 2.4.6 ab79c54 [Francesc Alted] Require numexpr 2.6.2
4 participants
git diff upstream/master | flake8 --diffThis addresses #15213. Actually, the recommended version for me is 2.6.2, but I understand that this is too recent still.