Handle deprecation of numpy _validate_lengths#3556
Handle deprecation of numpy _validate_lengths#3556soupault merged 1 commit intoscikit-image:masterfrom
_validate_lengths#3556Conversation
jni
left a comment
There was a problem hiding this comment.
@sciunto I've made a minor suggestion.
My personal preference, though, would be to vendor the code for _as_pairs. The leading underscore means that NumPy is under no obligation to keep that function stable, so we are just setting ourselves up for another bad surprise in the future.
skimage/util/arraycrop.py
Outdated
| from numpy.lib.arraypad import _as_pairs | ||
| except ImportError: | ||
| from numpy.lib.arraypad import _validate_lengths | ||
| old_numpy = True |
There was a problem hiding this comment.
@sciunto, I prefer to have the NumPy version check explicit here:
from distutils import LooseVersion as Version
old_numpy = Version(np.__version__) < Version('1.16')
if old_numpy:
from numpy.lib.arraypad import _validate_lengths
else:
from numpy.lib.arraypad import _as_pairs
This makes it clearer (though code) whether 1.15 is inclusive or exclusive.
There was a problem hiding this comment.
it's distutils.version import... ;)
|
I agree with your observation. There are pro and cons. numpy does the job to maintain this code vs they may change again the behavior. I do not have strong feelings on this. my position was to fix it asap first. |
A very good position! =) |
_validate_lengths
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free to suggest an improvement. |
|
Thanks @sciunto ! |
Handle deprecation of numpy `_validate_lengths`
Handle deprecation of numpy `_validate_lengths`
….14.x Merge pull request #3556 from sciunto/bugfix-3551
|
Any info on when this will get released on pypi? Numpy 1.16 just came out and broke this in our CI jobs. We can do a custom build of scikit-image internally or just pin numpy's version back for the time being but I prefer to keep with the mainstream of packages when possible |
|
@jriddy sorry about that! We are aiming to release 0.14.2 and 0.15 (Python 3.5+ only) in the next couple of weeks... |
|
@jni No reason to be sorry, we appreciate the work you guys do. Is the backport branch for 0.14.x currently stable for some value of that word? I can easily clone this and cut an internal 0.14.2-dev1 release that can hold us over until you guys release to PyPI. I just want to know if your dev cycle includes a big QA blitz at the end before a release, or if you generally expect things to work well in your backport branch. |
|
Oh! Both v0.14.x and master should work great! We try not to merge PRs to either that will break things or require follow up work. (Other than maybe code style or minor speed issues, and even then only rarely.) The last minute pre-release push is just fiddling with release notes and version numbers. |
|
Hmmm, is there any guide here for building wheels for this project? Locally my wheel is working, but inside my dockers I'm getting I'm building in a virtualenv on Ubuntu 18.04 with python 3.6.7. |
|
On Mon, 14 Jan 2019 08:33:28 -0800, Josh Reed wrote:
Hmmm, is there any guide here for building wheels for this project?
Locally my wheel is working, but inside my dockers I'm getting `E ImportError: /usr/local/lib/python3.6/site-packages/skimage/restoration/_unwrap_1d.cpython-36m-x86_64-linux-gnu.so: undefined symbol: PyFPE_jbuf` issues.
I'm building in a virtualenv on Ubuntu 18.04 with python 3.6.7.
The wheel building process is a bit complicated; that's what we have
this repo for: https://github.com/scikit-image/scikit-image-wheels
That said, I just noticed that our daily wheels have not been building
for a while, otherwise I could just have pointed you to those:
https://travis-ci.org/scikit-image/scikit-image-wheels/
@matthew-brett Do you know whether [this
error](https://travis-ci.org/scikit-image/scikit-image-wheels/jobs/479187321#L4177)
is currently present for all multibuilds?
|
|
@jriddy I'm not advocating leaving pypi behind, but scikit-image on conda-forge has the necessary patches to allow a 0.14.1 to be compatible with numpy 1.16. https://github.com/conda-forge/scikit-image-feedstock/tree/master/recipe If you can manage to build wheels for 0.14.1, then you should be able to use those patches to get the compatibility you need. |
|
It's okay, my original idea was to just mod the version and build a Thanks for your responses, folks! I really appreciate it. |
|
Hello. I am confused. I thought this is fixed in 0.14.2 but this PR is milestoned to 0.14.3. |
|
Because I renamed the milestone as several issues were still open after 0.14.2. I shouldn't have done that... Sorry. |
Description
Fixes #3551
I had to make a choice here. I preferred to keep using numpy. I removed the commented code (IMHO, not such a good practice to do that, we can browse numpy's history at any time)
A TODO task is added to remove the conditional function call.
This PR needs to be backported.
Checklist
[It's fine to submit PRs which are a work in progress! But before they are merged, all PRs should provide:]
./doc/examples(new features only)./benchmarks, if your changes aren't covered by anexisting benchmark
[For detailed information on these and other aspects see scikit-image contribution guidelines]
References
[If this is a bug-fix or enhancement, it closes issue # ]
[If this is a new feature, it implements the following paper: ]
For reviewers
(Don't remove the checklist below.)
later.
__init__.py.doc/release/release_dev.rst.@meeseeksdev backport to v0.14.x