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

Add example for fetching mutating datasets #144

Merged
merged 4 commits into from
Mar 1, 2020
Merged

Add example for fetching mutating datasets #144

merged 4 commits into from
Mar 1, 2020

Conversation

lmartinking
Copy link
Contributor

This is a bit of a hack, but you can fetch mutating datasets by using mock.ANY as the hash value.
Combined with daily sub directories, you can fetch data that changes potentially every day.
This PR adds a simple example of this scenario. I have discussed with @leouieda on slack.

This is a bit of a hack, but you can fetch mutating datasets by using `mock.ANY` as the hash value.
Combined with daily sub directories, you can fetch data that changes potentially every day.
This PR adds a simple example of this scenario.
@welcome
Copy link

welcome bot commented Feb 26, 2020

💖 Thanks for opening this pull request! 💖

Please make sure you read our contributing guidelines and abide by our code of conduct.

A few things to keep in mind:

  • Remember to run make format to make sure your code follows our style guide.
  • If you need help writing tests, take a look at the existing ones for inspiration. If you don't know where to start, let us know and we'll walk you through it.
  • All new features should be documented. It helps to write the docstrings for your functions/classes before writing the code. This will help you think about your code design and results in better code.
  • No matter what, we are really grateful that you put in the effort to do this! 🎉

doc/usage.rst Outdated
@@ -138,6 +138,37 @@ Alternative hashing algorithms supported by :mod:`hashlib` can be used if necess
print(pooch.file_hash("data/c137.csv", alg="sha512"))


Fetching Mutating Data
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Fetching Mutating Data
Bypassing the hash check

Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

@lmartinking thank you for taking the time to do this! I really appreciate it 🥇

I think this would be better as a general example of how to bypass the hash check. What I find missing a bit is explaining why it wouldn't work normally and how you got around that with mock.ANY (really smart hack by the way). I really like the setup and example so we could phrase it as:

Sometimes we might not know the hash of the file or it could change on the server periodically. In these cases, we need a way of bypassing the hash check. One way of doing that is with Python's unittest.mock module ... (yada yada yada).

In this example, we are downloading...

doc/usage.rst Outdated Show resolved Hide resolved
* Change section title
* Change description text per feedback
@lmartinking
Copy link
Contributor Author

Hey @leouieda I've updated the branch based on your feedback. Let me know what you think.

Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @lmartinking! I made some editorial suggestions to the text. You can commit them directly to your branch through Github. Feel free to reject/edit any of those 🙂

doc/usage.rst Outdated Show resolved Hide resolved
doc/usage.rst Outdated Show resolved Hide resolved
doc/usage.rst Outdated Show resolved Hide resolved
doc/usage.rst Outdated Show resolved Hide resolved
doc/usage.rst Outdated Show resolved Hide resolved
doc/usage.rst Show resolved Hide resolved
doc/usage.rst Outdated Show resolved Hide resolved
doc/usage.rst Outdated Show resolved Hide resolved
Co-Authored-By: Leonardo Uieda <leouieda@gmail.com>
@lmartinking
Copy link
Contributor Author

Hi @leouieda I have committed all your suggestions and then squashed them together. I think you write very clearly, I'm afraid you have ended up pretty much writing these docs as well!

@leouieda
Copy link
Member

leouieda commented Feb 28, 2020

@lmartinking not at all! This would have never happened if you hadn't started it. I just provided some tweaks to what you already had 🙂

Thank you for this contribution! We started a new authorship policy recently and would like to acknowledge all contributors by adding them as authors on the Zenodo archives we make for releases. We would be more than happy to include you if you could add yourself to the AUTHORS.md file (full name, affiliation, ORCID). You can do that in this PR still. Of course, that is optional and I understand if you'd rather not be included.

Either way, I'm happy with this and can merge as soon as the CI checks are green. 🎉

@lmartinking
Copy link
Contributor Author

@leouieda If I contribute code, then we can think about the authors file. Otherwise, I'm happy as is. Merge if ready! 😄

@leouieda
Copy link
Member

leouieda commented Mar 1, 2020

Alright, as you wish. Just wanted to point out that we value code and documentation contributions equally (what's the point of undocumented code?). Feel free to open a PR later adding yourself if you change your mind 🙂

Thank you!

@leouieda leouieda merged commit 529833e into fatiando:master Mar 1, 2020
@welcome
Copy link

welcome bot commented Mar 1, 2020

🎉🎉🎉 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, make sure that you have added your full name, affiliation, and ORCID (optional) to the AUTHORS.md file of this repository.

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.

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.

2 participants