fatiando / tutorials

Tutorials that integrate the Fatiando a Terra software to solve data problems in geoscience
https://www.fatiando.org/tutorials/
Other
13 stars 6 forks source link

Use Ensaio to fetch the tutorial data #3

Closed MGomezN closed 2 years ago

MGomezN commented 2 years ago

Fixes #2 Include a cell using Ensaio. Then, a Markdown section explaining that this could be done with Pooch to fetch any dataset.

Reminders:

welcome[bot] commented 2 years ago

💖 Thank you for opening your first pull request in this repository! 💖

A few things to keep in mind:

No matter what, we are really grateful that you put in the effort to do this!

review-notebook-app[bot] commented 2 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

MGomezN commented 2 years ago

Hello! Not sure if I have to "Run make format and make check" for this contribution. However, I tried conda install -c anaconda make getting the PackagesNotFoundError. Also, it seems not available for Windows. Anyways, if necessary, I will search for a solution :)

santisoler commented 2 years ago

This is looking great! Thanks @MGomezN ! :tada:

Thanks for adding ensaio to the environment.yml file and for making the changes in the notebook.

The only thing I would recommend is to rerun the entire notebook and then push it with all the outputs. You can always use the Run>Restart kernel and Rerun All Cells option.

Not sure if I have to "Run make format and make check" for this contribution.

The checklist in the PR is inherited from fatiando/.github and it doesn't apply to every repo, being tutorials one of them. Sorry for that. I would only keep the one that reminds you to add yourself to the AUTHORS.md file.

However, I tried conda install -c anaconda make getting the PackagesNotFoundError. Also, it seems not available for Windows. Anyways, if necessary, I will search for a solution :)

You're right, apparently Anaconda dropped the make binaries for Windows (they used to provide them). But, the good news is that conda-forge does provide them: https://anaconda.org/conda-forge/make

So, in order to install make on Windows you should run: conda install -c conda-forge make

Please, refresh my memory. Is there any place where we instruct people on installing make this way? It would be nice to update those instructions if we are providing them.

leouieda commented 2 years ago

@MGomezN thank you for making the change! I agree with @santisoler, this looks great and just needs a re-run.

Actually, there is a very serious mistake that needs to be fixed immediately. Your name is missing from the author list 😉

leouieda commented 2 years ago

About the checklist, I had opened a PR to make the organization wide template more general but it got lost in the holiday season: fatiando/.github#11

@MGomezN would you mind taking a quick look at that PR to see if it would be a helpful template for a general pull request? Is there anything missing that you would have liked to be reminded when you open one?

MGomezN commented 2 years ago

Thanks for your comments!

@MGomezN thank you for making the change! I agree with @santisoler, this looks great and just needs a re-run.

Deliberately I didn't run the cells for two reasons: 1) My awful/confusing paths: C:\Users\maria\AppData\Local\ensaio\ensaio\Cache\v1\southern-africa-gravity.csv.xz 2) By including outputs, I got "4,795 additions, 49 deletions not shown because the diff is too large." But I know I only made minor changes to the code, so numbers show no relevant differences. Therefore, I tried Jupytext. I generated a .py version of the notebook where only the appropriate changes to the code were reported and not all the info about the images in the outputs. But I still haven't perfectly understood how it works. That is why I erased the .py.

Now re-run is done

MGomezN commented 2 years ago

Please, refresh my memory. Is there any place where we instruct people on installing make this way? It would be nice to update those instructions if we are providing them.

Sorry, I thought it was in the Contributing Guidelines, but it is no, actually, under the phrase " Most repositories will also have a Makefile" the link is broken. Maybe I got that command for one of the multiple options we tried when setting up my Windows machine to work with Boule.

Happily,conda install -c conda-forge make Was successful.

leouieda commented 2 years ago

Thanks, @MGomezN! You're right that it's not nice to have our personal paths in the notebook like that. I'm not sure if we can have a better solution right now apart from making this into a Jupyter Book or something like that. I opened #4 to discuss this.

Merging this in!

welcome[bot] commented 2 years ago

🎉 Congrats on merging your first pull request and welcome to the team! 🎉

If you would like to be added as a author on the Zenodo archive of the next release, add your full name, affiliation, and ORCID (optional) to the AUTHORS.md file of this repository. Feel free to do this in a new pull request if needed.

We hope that this was a good experience for you. Let us know if there is any way that the contributing process could be improved.