Add functionality for setting custom names #361
Conversation
marcellevstek
force-pushed
the
modify/download
branch
5 times, most recently
from
49152bd to
914fd71
Compare
There was a problem hiding this comment.
Great work, PR looks ready - technical wise. However, I have second thoughts regaridn the arguments. One way to provide custom name is the way you did:
Data.download(
field_name="bam",
download_dir="/my/dir/",
custom_file_name="foo.txt"
)
The other option would be to make a breaking change:
Data.download(
field_name="bam",
target="/my/dir/foo.txt", # If one just wants to specify dir, target= "/my/dir/"
)
The naming target, resembles Path.rename, we could also use dst as in os.rename. What do you think? Do you have any opinion @gregorjerse ?
|
@JureZmrzlikar I agree with you about making the breaking change. I didn't want to incorporate it in the current implementation, but it makes complete sense not to use two distinct arguments for constructing the path. @gregorjerse please review. |
marcellevstek
force-pushed
the
modify/download
branch
2 times, most recently
from
43ad5bb to
b9661da
Compare
|
@gregorjerse I have added a new function for downloading and renaming a single file. Can you please guide me how to create a test, as I have problem mocking the download function. |
src/resdk/resources/data.py
Outdated
| ) | ||
| return | ||
|
|
||
| file_names = self.download( |
There was a problem hiding this comment.
Could be called by constructing kwargs. @gregorjerse if current implementation is not okay, I can change it.
src/resdk/resources/data.py
Outdated
| ) | ||
| if len(file_names) != 1: | ||
| raise ValueError( | ||
| f"Expected one file to be downloaded, but got {len(file_names)}" |
There was a problem hiding this comment.
Here the files have already been downloaded and are left in the download folder. What to do with them?
src/resdk/resources/data.py
Outdated
| raise ValueError( | ||
| f"Expected one file to be downloaded, but got {len(file_names)}" | ||
| ) | ||
| og_file_name = file_names[0] |
There was a problem hiding this comment.
What does og_file_name stand for?
There was a problem hiding this comment.
Original file name that is present on the platform.
marcellevstek
force-pushed
the
modify/download
branch
3 times, most recently
from
049e24c to
e920b10
Compare
marcellevstek
force-pushed
the
modify/download
branch
from
bc113e7 to
7e07be5
Compare
of downloaded files
REID-2630