-
Notifications
You must be signed in to change notification settings - Fork 3
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
Raise error if local_path
or central_path
does not point to project_name
folder.
#183
Conversation
22cdb28
to
0de6c00
Compare
local_path
or central_path
to not point to project_name
folder.
0de6c00
to
6f473b5
Compare
local_path
or central_path
to not point to project_name
folder.local_path
or central_path
does not point to project_name
folder.
6f473b5
to
217cf9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @JoeZiminski. I agree with the choice you've made here. I was convinced by your arguments for choosing this way over the alternative. The implementation seems to work.
I only left one minor comment concerning the example paths in the documentation.
Feel free to merge this.
c539ba1
to
31bc584
Compare
Co-authored-by: Niko Sirmpilatze <[email protected]>
Co-authored-by: Niko Sirmpilatze <[email protected]>
Thanks a lot for the suggestions @niksirbi ! |
closes #180.
This PR enforces the
local_path
andcentral_path
that the user inputs to themake_config_file
to point to a folder with the same name as a the datashuttleproject_name
. This also required fixing a lot of tests which did not use this pattern.Previously there was some discussion on the path that is passed to
make_config_file
. Currently it should be the folder path including the project folder name. This was also documented incorrectly.The two would be to to fix the
local_path
andcentral_path
to include the project-folder name, or fix the folder above and infer the project folder from the name of the initialised datashuttle project. After some back and forth (with myself) in this PR I went for the former for the below two reasons:Under the hood, the 'local_path' and 'central_path' should intuitively point to the project folder. Then searches are simply
local_path / rawdata
rather thanlocal_path / project_name / "rawdata"
. The latter could be added as a getter-function but is an extra level of complexity / maintenance that provides no benefit. It provides no benefit because for an instantiated project, we never want to switch between project folders, and so the configlocal_path
should always point to the specific project folder.It adds a level of robustness, in that it forces the user to input a path that explicitly matches the project_name they have input. We could search dynamically that the correct folder name exists in a root directory, but it seems easier to nip this in the bud at the offset and ensure everything is already aligned at the level of the config file.
Would be good to hear if this makes sense!