allegroai / clearml

ClearML - Auto-Magical CI/CD to streamline your AI workload. Experiment Management, Data Management, Pipeline, Orchestration, Scheduling & Serving in one MLOps/LLMOps solution
https://clear.ml/docs
Apache License 2.0
5.69k stars 655 forks source link

Add functionality to add files from a list of paths #1129

Closed CharlesFrankum closed 8 months ago

CharlesFrankum commented 1 year ago

Related Issue \ discussion

https://github.com/allegroai/clearml/issues/1127

Patch Description

Adds logic to _add_files so that we can now additionally pass in a list of paths

Testing Instructions

from os import listdir
from os.path import isfile, join

from clearml import Dataset

test_dir = ''
file_paths = [f for f in listdir(test_dir) if isfile(join(test_dir , f))]
dataset.add_files(paths=paths)
eugen-ajechiloae-clearml commented 1 year ago

Hi @CharlesFrankum ! I don't think we should have a new function here, but instead make path in add_files support sequences as well, similarly to how it's done in add_external_files: https://github.com/allegroai/clearml/blob/18210ed144ce630fb1bda44d07cea356a7e23b53/clearml/datasets/dataset.py#L436

CharlesFrankum commented 1 year ago

Hi @CharlesFrankum ! I don't think we should have a new function here, but instead make path in add_files support sequences as well, similarly to how it's done in add_external_files:

https://github.com/allegroai/clearml/blob/18210ed144ce630fb1bda44d07cea356a7e23b53/clearml/datasets/dataset.py#L436

Ah ok! I wasn't sure if we wanted to accept multiple types here but makes sense - will make the changes 👍

jkhenning commented 1 year ago

Hi @CharlesFrankum , any update?

eugen-ajechiloae-clearml commented 8 months ago

Hi @CharlesFrankum ! Why was this PR closed? Any way we can help reopen it?

CharlesFrankum commented 8 months ago

Hi @CharlesFrankum ! Why was this PR closed? Any way we can help reopen it?

Ah sorry, I messed up my master branch on the fork so reset it so I could work on a different PR and it automatically closed this PR. I can reopen it and work on it in a little. Sorry for never finishing this one btw - we ended up moving away from clearml datasets for now so was low priority

eugen-ajechiloae-clearml commented 8 months ago

@CharlesFrankum It would be great if we could have this PR merged. If you could reopen it that would be great. If you don't wish to work on it anymore I can take over.