-
Notifications
You must be signed in to change notification settings - Fork 649
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
Relative path is incorrect when using Dataset.add_external_files
for local file without the file://
prefix
#1313
Comments
Hi @geometrikal ! Thanks for reporting this. We will get back to you once this is fixed. |
Hi @eugen-ajechiloae-clearml i would like to work on this. Can i pick it up. I am new to the repo, can you please help with some guidance on what has to be done here. |
Hi @d-vignesh ! Yes, all contributions are welcome, and no-one picked this one up as far as I'm aware. clearml/clearml/datasets/dataset.py Line 3322 in b1e39e6
I would add a check: if the file is a local path without file:// then get the absolute path and add the file:// prefix.This should be tested using absolute paths, relative paths, on both Windows and Linux/MacOS. |
Hi @eugen-ajechiloae-clearml can be please clarify on below doubt,
for example, if the source_url is |
Hi @d-vignesh ! There are 2 cases when adding external files: you can either add a "directory" or a direct path. In case the source URL is the direct path, we fallback to |
thank you @eugen-ajechiloae-clearml , got it. Please verify my understanding, that user can give both absolute path |
Hi @eugen-ajechiloae-clearml , created a PR for the fix #1326. Can you please have a look at it. |
Describe the bug
Dataset.add_external_file
will allow adding external files from a source url without thefile://
prefix, e.g./path/to/my/file.csv
. It will create the link correctly in the metadat.json by prependingfile://
however it will remove characters from the file name when creating the relative link.To reproduce
Expected behaviour
Either throw an exception or correctly create the relative path
Environment
The text was updated successfully, but these errors were encountered: