Skip to content

Conversation

@kakirastern
Copy link
Contributor

@kakirastern kakirastern commented Feb 26, 2019

Update some old .ipynb tutorial notebooks.

Fixes #335.

@kakirastern
Copy link
Contributor Author

Think I might need to turn off caching of downloaded files for the tutorials, and then add the comment that cache = False can be modified to cache = True if we do not want to see the timeout or URLErrors. Will need to go through the tutorials one-by-one for that.

@adrn
Copy link
Contributor

adrn commented Feb 28, 2019

I like the sentiment of the title, but I'm a little confused about the changes you've made: can you explain a bit? e.g., I don't think changing "HCG 7" to "hcg7" will fix the broader issues. I do, however, agree that we should review each notebook, identify places where they try to talk to the internet, and decide whether we want to keep those instances or replace them with cached values. I could imagine, for example, replacing:

hcg7_center = SkyCoord.from_name('hcg7')

with something like:

# This line accesses the internet - if you have an internet connection, try this out, 
# otherwise use the default cached values included below this line:
# hcg7_center = SkyCoord.from_name('hcg7')
hcg7_center = SkyCoord(ra=..., dec=...)

@kakirastern
Copy link
Contributor Author

kakirastern commented Mar 1, 2019

I originally thought it shouldn't make a difference between the queries "HCG 7" and "hcg7" too, but after testing a bit I found that somehow Travis CI thinks it does... (Maybe a long time ago there used to be no difference?) If I don't change this occasionally an error would be thrown due to the syntax. I will add the few lines of explanation you gave in #341 (comment) however, to clarify the situation a bit more. I will test it again locally as well to confirm.

@kakirastern kakirastern changed the title Update old .ipynb tutorial notebooks to pass Travis CI Update old .ipynb tutorial notebooks to pass Circle/Travis CI Mar 10, 2019
@adrn
Copy link
Contributor

adrn commented Mar 10, 2019

@kakirastern I think the "HCG 7" issue is a red herring: try it locally, you'll see that the name "HCG 7" resolves just fine

With astropy v3.1:

>>> from astropy.coordinates import SkyCoord
>>> SkyCoord.from_name("HCG 7")
<SkyCoord (ICRS): (ra, dec) in deg
    (9.81625, 0.88805556)>

I'm going to have a go at making some changes to solve #348, so this might not get merged in the end, but I'll keep this open for now. Thanks!

@kakirastern
Copy link
Contributor Author

Yup, no problem @adrn... Have updated #341 (comment) accordingly.

@kakirastern kakirastern changed the title Update old .ipynb tutorial notebooks to pass Circle/Travis CI Update old .ipynb tutorial notebooks Mar 10, 2019
@kakirastern
Copy link
Contributor Author

kakirastern commented Mar 10, 2019

I think the "HCG 7" issue is a red herring: try it locally, you'll see that the name "HCG 7" resolves just fine

Yes, it is strange. I did try it locally and I remember the term "HCG 7" did resolve just fine... Not sure why Circle CI is getting so sensitive.

@eblur
Copy link
Contributor

eblur commented Jun 25, 2019

I'm closing this PR because the original issue was handled in PR #370

@eblur eblur closed this Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add TODO list items to Quantities tutorial

3 participants