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

Improve docstring of known_hash in retrieve function #333

Merged

Conversation

BjoernLudwigPTB
Copy link
Contributor

@BjoernLudwigPTB BjoernLudwigPTB commented Jan 17, 2023

Specify that known_hash can also be None in the retrieve function
docstring. This change helps some IDEs to identify None as a valid value for
the known_hash argument.

My IDE complains about None as being not allowed as described in the corresponding issue.

Relevant issues/PRs:

Fixes #332.

@welcome
Copy link

welcome bot commented Jan 17, 2023

💖 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!

@BjoernLudwigPTB
Copy link
Contributor Author

I am a bit surprised about the single pipeline failure. As I did not touch any code, I can hardly believe those failures are really related to my very minor change. Or is there something I missed?

@santisoler
Copy link
Member

Thanks @BjoernLudwigPTB for this contribution! Don't worry, your change cannot trigger any test failure. Apparently the downloaded file got corrupted and the hash didn't matched, which proves that including checksums in Pooch was a great idea from the start. I rerun it. Let's wait until it passes and I'll merge this PR 🚀 .

@santisoler santisoler changed the title Add 'or None' to retrieve docstring's known_hash type annotation Improve docstring of known_hash in retrieve function Jan 19, 2023
@santisoler santisoler merged commit 76897d5 into fatiando:main Jan 19, 2023
@welcome
Copy link

welcome bot commented Jan 19, 2023

🎉 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.

@BjoernLudwigPTB
Copy link
Contributor Author

@santisoler Say, am I really invited, for such a small contribution, to join the AUTHORS.md? I would love to, but it feels a bit awkward as I imagine, most if not all the others did actually contribute much more useful things.

@BjoernLudwigPTB BjoernLudwigPTB deleted the docs/correct_retrieve_docstring branch January 20, 2023 15:07
@santisoler
Copy link
Member

Yes @BjoernLudwigPTB! Every person that makes a contribution to our packages is automatically a contributor, so your GitHub handle and/or name will appear in our changelog. Every contributor can opt-in to be in the AUTHORS.md. This is completely optional and the addition should be done by the contributor themselves. The contributors listed in the AUTHORS.md file will appear in the Zenodo releases we make of our packages (like the one for Pooch) and can also be a candidate to be an author of any scientific paper for that particular package (like the Verde paper for example). More details on that in our Authorship guidelines.

Feel free to add yourself if that's what you desire. If not, don't worry. It's ok to opt-out if you aren't interested in that. You contribution and any future ones will still be very welcomed!

BjoernLudwigPTB added a commit to BjoernLudwigPTB/pooch that referenced this pull request Jan 26, 2023
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.

Type annotation for retrieve's known_hash should be str or None
2 participants