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

Fix bug when unpacking a subfolder from an archive #266

Merged
merged 7 commits into from
Oct 11, 2021
Merged

Conversation

leouieda
Copy link
Member

@leouieda leouieda commented Oct 1, 2021

Our unpacking processors (Untar and Unzip) aren't handling members that are a whole subfolder correctly. The elements of members are being treated as files regardless of what they are. Add a test for use case (no change to our test data required) and refactor the unpacking test to make it easier to extend in the future.

Fixes #264

Reminders:

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst and the base __init__.py file for the package.
  • Write detailed docstrings for all functions/classes/methods. It often helps to design better code if you write the docstrings first.
  • If adding new functionality, add an example to the docstring, gallery, and/or tutorials.
  • Add your full name, affiliation, and ORCID (optional) to the AUTHORS.md file (if you haven't already) in case you'd like to be listed as an author on the Zenodo archive of the next release.

It was a bit too complicated with too many parameters. Managed to
simplify it a bit.
Tests fail with the expected error message.
Added a test case for multiple members (one being a subdir). The
solution actually leads to simpler code than what we originally had.
Pretty much the same solution
Move the checking of logs and logic for expected paths and log messages
to their own functions.
@leouieda leouieda marked this pull request as ready for review October 1, 2021 11:22
@leouieda
Copy link
Member Author

leouieda commented Oct 3, 2021

@adam2392 this should fix the issue. Would you be able to try using the version in the unpack-bug branch to see if it works for you?

@leouieda
Copy link
Member Author

Thanks for the review @hugovk 👍🏽 Merging this and making a new release. Hopefully nothing will break upstream this time 🤞🏽

@leouieda leouieda merged commit 3f6d065 into master Oct 11, 2021
@leouieda leouieda deleted the unpack-bug branch October 11, 2021 08:27
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.

Untarring members that are sub-directories themselves results in an error
2 participants