seedcase-project / seedcase-sprout

Upload your research data to formally structure it for better, more reliable, and easier research.
https://sprout.seedcase-project.org/
MIT License
0 stars 0 forks source link

feat: ✨ add `create_dir()` and `create_dirs()` with tests #586

Closed K-Beicher closed 2 weeks ago

K-Beicher commented 3 weeks ago

Description

These changes are done to make the internal function that creates directories.

Closes #566

Reviewer Focus

This PR needs an in-depth review. The first function I've made, I'm fairly sure I've done it correctly.

Checklist

K-Beicher commented 3 weeks ago

@seedcase-project/developers Just thinking about this function, and what it should do, if it encounters a directory that already exists.

1/ does it go back and informs the user that it is already there (and may already contain data)

2/ does it just confirm that the directory is created, and ignores the fact that it wasn't created now.

This becomes a bit more tricky if we are talking about more than one directory (as Luke's mod allows), what happens if one of several directories exists.

1/ goes back and informs about the one that is already there and then: 1a/ doesn't create the rest of them 1b/ creates the rest of the directories

2/ creates the new directories and confirms that all directories are now in place.

PS We need more tests if it is to run more than one directory at a time.

lwjohnst86 commented 3 weeks ago

To answer the first question, this is an internal function so no user interaction is expected so we don't need to inform on anything.

My feeling is, if any directory already exists, it throws an error about creating it so no directories are created in the end. Which would mean we would need to run verify_is_dir() before actually creating the dirs.

K-Beicher commented 3 weeks ago

@signekb I think I've got the error handling to work now.