vsoch / nidmviewer

NIDM Results Viewer

http://vsoch.github.io/nidmviewer/

BSD 3-Clause "New" or "Revised" License

3 stars 3 forks source link

python 3 support #20

Open vsoch opened 6 years ago

vsoch commented 6 years ago

wow, must have made this a long time ago :P

chrisgorgo commented 6 years ago

Indeed does not seem to be Python 3 compatible:

File "/usr/local/lib/python3.6/site-packages/nidmviewer/viewer.py", line 5, in <module>
    from nidmviewer.utils import strip_url, get_random_name, get_extension, \
  File "/usr/local/lib/python3.6/site-packages/nidmviewer/utils.py", line 63
    print "Cannot download %s" %(src)

vsoch commented 6 years ago

I can get this fixed up later tomorrow (after moving truck), thanks for letting me know!

abitrolly commented 6 years ago

@vsoch I can help you with truck moving.

vsoch commented 6 years ago

lol, I appreciate the offer, but you might have a hard time getting here to help from Belarus! I should be ok :)

vsoch commented 6 years ago

ohh hmm, it looks like I did this months ago and it was never reviewed by @satra and @cmaumet ... https://github.com/vsoch/nidmviewer/pull/24

I knew I wasn't nuts :P

abitrolly commented 6 years ago

Yes, I hoped for a free ticket. )

Maybe splitting #24 into several logical PRs could put less burden and more fun on reviewers. Licenses are surely the most complicated thing.

vsoch commented 6 years ago

This is the license I've chosen to ensure the software remains open source. AGPL basically says you are free to use the software as long as any changed code remains public, and unchanged could still be private. NeuroValt would not have issue, per my understanding of the license. Codewise is not too complicated to review. Could you explain the issue that you have?

I'm also going back to sleep, it's 3:30 am here :)

vsoch commented 6 years ago

Just kidding it's BSD and not AGPl, which is very permissive.

vsoch commented 6 years ago

@abitrolly per my suggestion in the PR I think this would be best tackled by writing tests. Would you care to take a shot?

abitrolly commented 6 years ago

I have no idea how to setup fixtures/environment for testing it (HTTPS redirects etc.). There should be declarative format to describe such setups in virtual environment, but I don't know any, and before writing one I better fund a job to cover human body possession expenses while I complete all other tools in the queue. :)

vsoch commented 6 years ago

@chrisfilo your thoughts?

chrisgorgo commented 6 years ago

Not sure what you are talking about @abitrolly - care to elaborate?

I agree that splitting #24 into three PRs (one for fixing each of #19 #23 and #20) - I see @cmaumet also gave a thumbs up for that. It would also make it easier to see which parts of the code need tests. @eayoungs has committed to porting NeuroVault to python 3 which this is part of so they can hopefully help.

abitrolly commented 6 years ago

Nevermind. Splitting PRs into pieces is a way to go.

eayoungs commented 6 years ago

I'm catching up on the conversation, now...

vsoch commented 6 years ago

Here is the first PR, please read the description for... exactly that :) https://github.com/vsoch/nidmviewer/pull/25 When this goes through I will start on the next.

abitrolly commented 6 years ago

25 is there, so what is the next?