Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Geosci labs #111

Merged
merged 13 commits into from
Aug 7, 2019
Merged

Geosci labs #111

merged 13 commits into from
Aug 7, 2019

Conversation

lheagy
Copy link
Member

@lheagy lheagy commented May 14, 2019

Follow the geosci-labs repository for maintaining the notebooks. The maintenance strategy now follows that outlined in geosci-course

👋 @thast, would you be willing to review this?

@lheagy lheagy requested a review from thast May 15, 2019 01:00
@thast
Copy link
Member

thast commented May 17, 2019

Warning on mybinder. I will add it in this PR:
image

Copy link
Member

@thast thast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @lheagy , trying to run this branch on mybinder, none of the seismic apps are working (error 404: page not found). Same for the EM_ThreeLoopModel, and the EM_ResponseFunction.

It might be just a naming problem (for example, it seems EM_ThreeLoopModel got renamed FDEM_ThreeLoopModel; for seismic the path is now seismic/....ipynb and not anymore seis/....ipynb.

Can you fix those paths issues? Thanks.

I have added the pip dependency that mybinder was "nagging" me about, but it looks weird. Check and let me know if you have a better option.

Also: We need to check why Travis did not picked up those issues.

@lheagy
Copy link
Member Author

lheagy commented May 17, 2019

Many thanks @thast! I will get to this over the weekend. Does the overall nblibrarian workflow make sense to you?

@thast
Copy link
Member

thast commented May 17, 2019

I did not looked at nlibrarian too deeply yet. This is the jupyter-include file am I right? But I still see the notebooks being part of the gpgLabs repository on this branch. So I am unsure of what jupyter-include does. I get we install the code (*.py) from geosci-labs but not how the notebooks are updated automatically?

@lheagy lheagy mentioned this pull request Aug 6, 2019
@lheagy
Copy link
Member Author

lheagy commented Aug 6, 2019

👋 @thast: would you be willing to do another pass on this? I updated the links, so I believe it should be good to go unless you catch anything else. Thanks!

Copy link
Member

@thast thast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job @lheagy ! looks great!

Two comments:

  • in index.ipynb, the contact link is broken and needs to be updated (see picture below) (fixed)
  • at the root of the repository, is the uppercase necessary for the Notebooks folder? (see picture below)

image

image

@lheagy
Copy link
Member Author

lheagy commented Aug 6, 2019

Thanks @thast! It looks like you fixed the broken link :) and the notebooks directory is renamed in this pr to be lower-cased

image

@thast thast merged commit a8cd8de into master Aug 7, 2019
@thast thast deleted the geosci-labs branch August 7, 2019 00:07
@thast thast mentioned this pull request Aug 7, 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.

Mag_Induced2D crashes
2 participants