Repository navigation
Add requests to astropy/extern #1035
Description
requests would make web-related code in astropy easier to write / read / maintain. The question of whether to include it in astropy/extern came up in pull request #972 by @lpaioro and @mdboom was for including it:
I think it would simplify a number of things throughout astropy -- particularly in the forthcoming VO query stuff.
For astroquery we already decided to include it in extern here (which wouldn't be necessary any more if it were in astropy).
Here's some basic info:
- pure-Python, simultaneous Python 2 / 3 codebase
- it's quite a lot of files and 300 kB (because
requestsbundlesurllib3andcharade. grep import *.py */*.py */*/*.py | grep urlshows that web-related code is used at the moment in the following modules:coordinates/name_resolve.py,utils/data.py,utils/misc.py,io/fits/file.py,io/votable/tree.py,utils/xml/check.py... and soonvo/samp.
So should we include it?
Or would it be better / acceptable as an extra external dependency in addition to numpy (used only where needed like e.g. with scipy)?
Activity
embray commentedon Apr 30, 2013
Definitely +1 to using requests. When I first discovered it I was like, "wow, where have you been all my life?"
I'm sort of -1 on including it in extern. I would instead prefer to use requests where possible, and ask users to install it if not. I'm also not sure how we feel about adding install dependencies at this stage. I'd personally be all for just adding it to install_requires since it's not something we need already installed at installation time (like configobj) and it's pure python so there are no build-related issues.
eteq commentedon May 29, 2013
I am also 👍 for using
requests, but only if we are willing to use it consistently everywhere (e.g., #1123).As for including it in
extern, I think we have to if we're really going to use it. It would need to be part of low-level utilities likeastropy.utils.dataif we're really serious about it. The original vision document was that we not have any real dependencies other thannumpy... That's what motivatedexternandcexternin the first place. Packages likescipyandmatplotlibare "optional" because only some specific subpackages require them. But we couldn't do that withrequestsbecause of the need to have it in low-level utilities.Of course, we could have a discussion about re-visiting this and breaking out the
externpackages into install requirements. But that has even wider-ranging effects.Also, note kennethreitz/requests#1364 - that's problematic for adopting it until that's fixed. But once that happens, I'm inclined to say we should try to move everything that currently uses
urllib(2)over in one fell swoop, along with puttingrequestsinextern. You'll see from #1123 that this actually might be good for affiliated packages, too.astrofrog commentedon May 29, 2013
While I can understand using
requestsin astroquery, I'd like to see a good argument for using it in the core package. Exactly which parts of the core package will it affect and simplify? Not saying we shouldn't do it, just that I'm not aware at the moment of the benefits for the core package.keflavich commentedon May 29, 2013
@astrofrog - I think it simplifies any usage of
urllib#; mostly it seems that any usage ofurllib2requires some use ofurlliband it usually ends up being 1/4 the lines of more readable code when usingrequestsinstead. That might not be enough of a reason to use it in core, but that's been my experience so far.astrofrog commentedon May 29, 2013
Ok - I guess I want to be cautious with including too much in
externif it's not really needed. Certainly, beforeastropy.vo, there were (I think) two calls tourlopen, so going from 8 to 2 lines wouldn't really be worth it. But if it makes a lot of the VO and other functionality easier, then maybe we should indeed consider it.embray commentedon May 29, 2013
I think I've mentioned this elsewhere, but would would it be wrong to just add requests as an external dependency to be installed by the package installer? (that is
install_requires=['requests']in the setup.py). We haven't done this with some of the other things in extern because they're dependencies for some of the core functionality in Astropy like the config system. But I think we could do without requests until runtime, no?astrofrog commentedon May 29, 2013
So would it be a compulsory run-time requirement?
eteq commentedon Jun 6, 2013
I think it would have to be, because we would want the
datastuff to be based onrequests(if we use it at all in the core), and some of that is nominally fairly low-level. I think people would be pretty surprised if they couldn't get leap seconds right (as a near-future example), because they didn't haverequestsinstalled.cdeil commentedon Feb 26, 2014
I don't think there's a concrete plan to add / use
requestsin astropy.I'm closing this issue for now (too many old open issues IMO).
If someone thinks we should add / use
requestsin astropy, please comment here to have the issue re-opened.