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

Fast shm from scipy 0.15.0 does not work on rc version #622

Closed
samuelstjean opened this Issue Apr 2, 2015 · 21 comments

Comments

Projects
None yet
4 participants
@samuelstjean
Contributor

samuelstjean commented Apr 2, 2015

The strict version checker for the new shm function does not play well with non release version of scipy sadly, and everything breaks.

File "/home/mpizzola/workspace/modules/scilpy/scilpy/denoising/smoothing.py", line 8, in
from dipy.reconst.shm import sph_harm_ind_list, real_sph_harm, lazy_index
File "/user/mpizzola/home/workspace/modules/dipy/reconst/shm.py", line 43, in
if StrictVersion(scipy.version.short_version) >= StrictVersion('0.15.0'):
File "/home/mpizzola/Installations/canopy/appdata/canopy-1.2.0.1610.rh5-x86_64/lib/python2.7/distutils/version.py", line 40, in init
self.parse(vstring)
File "/home/mpizzola/Installations/canopy/appdata/canopy-1.2.0.1610.rh5-x86_64/lib/python2.7/distutils/version.py", line 107, in parse
raise ValueError, "invalid version number '%s'" % vstring
ValueError: invalid version number '0.14.1rc1'

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 2, 2015

The problem is that Marco has an invalid version number. Is that correct @matthew-brett ?

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Apr 2, 2015

He does have an invalid version number according to strict checking, but it is scipy's version number not Marco's. I suspect that all non-release scipy version strings (rc1, dev etc) previous to the current scipy development version will fail this check.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 2, 2015

Maybe the easiest fix would be that Marco upgrades to 0.14.1 or 0.15.0?
https://github.com/scipy/scipy/releases
Should we use LooseVersion to resolve this issue? What do you suggest?

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Apr 2, 2015

Is there any reason not to use LooseVersion? It is annoying to force people to upgrade scipy unless there is a good reason.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 2, 2015

I don't know. That is why I am asking you. If not we can then use LooseVersion.

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Apr 2, 2015

git blame says that Ariel put that check in - @arokem - any reason for the StrictVersion check in shm at line 43?

@arokem

This comment has been minimized.

Member

arokem commented Apr 2, 2015

No - no particular reason. Seems like I was trying to deal with something
similar:

4bce82e

We should probably do the same that we do in core.optimize (i.e.
LooseVersion).

On Thu, Apr 2, 2015 at 8:48 AM, Matthew Brett notifications@github.com
wrote:

git blame says that Ariel put that check in - @arokem
https://github.com/arokem - any reason for the StrictVersion check in
shm at line 43?


Reply to this email directly or view it on GitHub
#622 (comment).

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 2, 2015

Okay, great can you make a quick PR?

@arokem

This comment has been minimized.

Member

arokem commented Apr 2, 2015

Who is "you"?

On Thu, Apr 2, 2015 at 8:56 AM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

Okay, great can you make a quick PR?


Reply to this email directly or view it on GitHub
#622 (comment).

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 2, 2015

Dr. Ariel Rokem! The Cool Dude!

@arokem

This comment has been minimized.

Member

arokem commented Apr 7, 2015

Closed through #626? @samuelstjean ?

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented Apr 7, 2015

We should ask Marco, I was walking him through some code and everything broke in his face and I merely reported the issue.

@arokem

This comment has been minimized.

Member

arokem commented Apr 7, 2015

Yes - please do ask Marco.

On Tue, Apr 7, 2015 at 5:17 AM, Samuel St-Jean notifications@github.com
wrote:

We should ask Marco, I was walking him through some code and everything
broke in his face and I merely reported the issue.


Reply to this email directly or view it on GitHub
#622 (comment).

@arokem

This comment has been minimized.

Member

arokem commented May 2, 2015

Any news on this? Can we close?

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented May 2, 2015

Ask Marco, no idea if he has a github account though.

Le 2015-05-02 07:57, Ariel Rokem a écrit :

Any news on this? Can we close?


Reply to this email directly or view it on GitHub
#622 (comment).

@arokem

This comment has been minimized.

Member

arokem commented May 2, 2015

I don't know who Marco is.

On Sat, May 2, 2015 at 5:03 AM, Samuel St-Jean notifications@github.com
wrote:

Ask Marco, no idea if he has a github account though.

Le 2015-05-02 07:57, Ariel Rokem a écrit :

Any news on this? Can we close?


Reply to this email directly or view it on GitHub
#622 (comment).


Reply to this email directly or view it on GitHub
#622 (comment).

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented May 2, 2015

Oh right, I'll email then I guess.

Le 2015-05-02 08:08, Ariel Rokem a écrit :

I don't know who Marco is.

On Sat, May 2, 2015 at 5:03 AM, Samuel St-Jean notifications@github.com
wrote:

Ask Marco, no idea if he has a github account though.

Le 2015-05-02 07:57, Ariel Rokem a écrit :

Any news on this? Can we close?


Reply to this email directly or view it on GitHub
#622 (comment).


Reply to this email directly or view it on GitHub
#622 (comment).


Reply to this email directly or view it on GitHub
#622 (comment).

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented May 6, 2015

Well, he actually installled 0.15 in the meantime, so I'll try a virtualenv or something with a dev version I guess.

@arokem

This comment has been minimized.

Member

arokem commented May 6, 2015

Thanks for following up on this!

On Wed, May 6, 2015 at 8:33 AM, Samuel St-Jean notifications@github.com
wrote:

Well, he actually installled 0.15 in the meantime, so I'll try a
virtualenv or something with a dev version I guess.


Reply to this email directly or view it on GitHub
#622 (comment).

@arokem

This comment has been minimized.

Member

arokem commented Jul 29, 2015

Any conclusions here?

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented Jul 29, 2015

Seems good, it does not crash and gives the right thing

In [1]: from distutils.version import LooseVersion

In [2]: version = '0.14.1rc1'

In [3]: LooseVersion(version) >= LooseVersion('0.15.0')
Out[3]: False

In [4]: version = 'wathever-0.17'

In [5]: LooseVersion(version) >= LooseVersion('0.15.0')
Out[5]: True

@arokem arokem closed this Jul 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment