fatiando / pooch

A friend to fetch your data files
https://www.fatiando.org/pooch
Other
630 stars 76 forks source link

Ensure processor has extracted all members when members=None #365

Closed jni closed 9 months ago

jni commented 1 year ago

Here's an early attempt at fixing #364. Currently, test_unpacking is failing because of the log tests. I'm not 100% sure what to do about them because the log becomes pretty complex, so I don't know how to generate the expected log. Maybe this is the wrong approach — happy to revert the last commit and try something else if you have suggestions! 🙏

Commits

Relevant issues/PRs:

Fixes #364

welcome[bot] commented 1 year 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!

jni commented 1 year ago

👋

leouieda commented 1 year ago

Hi @jni sorry for the very very long delay on this. We'll try to put out the fire on #373 and then get on this.

leouieda commented 1 year ago

@jni I think the problem is that you're changing the value of self.members. There is a check in _extract_file for self.members is None to decide how to extract and that check is now always false. I think the fix would be to set a local members variable in __call__ that either is self._all_members(fname) or self.members.

jni commented 1 year ago

Thanks @leouieda! I'm sure I have older neglected PRs as a maintainer. 😂 And your suggestion seems to have done the trick! 🚀

jni commented 11 months ago

@leouieda icymi I made the suggested changes! 👆

leouieda commented 9 months ago

@jni sorry for the delay on this. I'll get these documentation issues fixed in main and then merge this asap. Thanks for your help!

jni commented 9 months ago

@leouieda don't worry, I am very comfortable with long-lived PRs. 😂 I appreciate your efforts! 🙏 Please let me know if I can do anything else.

welcome[bot] commented 9 months 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.

leouieda commented 9 months ago

Alright! I made some slight modifications to make ExtratorProcessor an abstract base class. That got rid of the need to check if methods are implemented and made the tests and checks pass with our new pylint configuration.

Thanks for the contribution @jni! ❤️