Fixes to find_api_page function #739
Conversation
astropy/utils/misc.py
Outdated
There was a problem hiding this comment.
Technically, urllib2 doesn't exist in Python 3, so wondering whether you should say 'will be used in Python 2, and the default for ... in Python 3'.
There was a problem hiding this comment.
Good point. I'll adjust this to just say something like "the python global default will be used" - the urllib documentation is equally inspecific about this anyway (on any version).
|
Apart from my comment above, this looks fine to me. Is a 1 second timeout not a little short? Maybe 5 or 10 seconds would make more sense? |
|
5-10 seems pretty long to wait for something that's intended as a quick convinience. But I suppose you're right that 1 might be problematic if you're not on a fast network or broadband or something. How about we split the difference and say 3? |
|
Oh, wait, I'm realizing now you're talking about the tests. I'd still say 3, because the main impetus was that it kept hanging on me when I ran the tests and I had to wait a long time. So by empirical tests of my own personal annoyance threshold, ~5 sec is the limit :) |
|
Ok, 3 is good ;-) Yes, I meant for the tests - it's be a shame to have the tests fail just because the network is slow for Travis. But let's see if it works. |
|
Alright, I'll go ahead and merge this now then. The tests aren't testing this anyway, seeing as how |
|
Why do people keep tagging fixes related to this function for 0.2? It's not even in 0.2. |
|
Because we keep forgetting it's not in 0.2 :) |
3 participants
this contains a py3.x fix to the
utils.misc.find_api_pagefunction, and an option for passing in a timeout. These were partly uncovered by #734, although they won't necessarily be tested here, given that the--remote-dataoption is not yet in the tests. (It does pass locally for me, though)