glm-tools / pyglmnet

Python implementation of elastic-net regularized generalized linear models
http://glm-tools.github.io/pyglmnet/
MIT License
279 stars 83 forks source link

ImportError with urllib, python2 #292

Closed cxrodgers closed 5 years ago

cxrodgers commented 5 years ago

pyglmnet currently cannot be imported under python 2 due to a syntax change in urllib. A simple change can resurrect python 2 functionality without affecting anything on python 3. I realize that we don't want to support python 2. Is it acceptable to include this fix?

All that has to happen is this change in datasets.py:

try:
    # Python 3
    from urllib.request import urlretrieve
except ImportError:
    # Python 2
    from urllib import urlretrieve

I realize this is a charged issue, I don't want to create controversy, just making a proposal. I will create a pull request as well. Sorry for duplication, just didn't know if discussion is better suited here on the Issue page or in the Pull Request page.

jasmainak commented 5 years ago

I would suggest upgrading to Python 3 :-) Properly supporting Python 2 is going to be problematic and most libraries have moved away from it.

cxrodgers commented 5 years ago

Yes, I would never suggest "properly supporting Python 2" for that reason. Just thought I would mention this simple change. I also understand that you might not want to set a precedent of accepting such changes. Worst case, this issue can be closed and just stay here as a helpful tip for any other Luddites out there who are, for whatever reason, trying to get it working on Python 2.

jasmainak commented 5 years ago

see this: https://github.com/glm-tools/pyglmnet/pull/286

I'm closing the issue for now.