-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
enhance download_and_extract #8216
Conversation
Hi @Jerome-Hsieh If you're having issues with putting this PR together we can discuss how to resolve them, but please do not close and then open a new PR. Thanks for the contribution! |
Signed-off-by: jerome_Hsieh <[email protected]>
Signed-off-by: jerome_Hsieh <[email protected]>
Signed-off-by: jerome_Hsieh <[email protected]>
0511db5
to
0441871
Compare
Signed-off-by: jerome_Hsieh <[email protected]>
@ericspod @KumoLiu |
Hi @Jerome-Hsieh, thank you for the quick update. I tested your latest changes, but neither "." nor "./test" as the file path seems to work. Is this the expected behavior? In my opinion, both should work. The logic could be adjusted so that if the file path is a directory and not empty, it automatically captures the name and downloads the file into the specified directory. What do you think?
|
Hi @KumoLiu my test results: >>> url = "https://github.com/Project-MONAI/MONAI-extra-test-data/releases/download/0.8.1/MedNIST.tar.gz"
>>> monai.apps.utils.download_and_extract(url, filepath=".")
2024-12-06 18:06:56,333 - INFO - Expected md5 is None, skip md5 check for file ..
2024-12-06 18:06:56,334 - INFO - File exists: ., skipped downloading.
2024-12-06 18:06:56,334 - INFO - Non-empty folder exists in ., skipped extracting. and >>> monai.apps.utils.download_and_extract(url, filepath="./test")
2024-12-06 18:08:41,246 - WARNING - filepath=./test, which missing file extension. Auto-appending extension to: ./test.tar.gz
test.tar.gz: 59.0MB [00:10, 6.07MB/s]
2024-12-06 18:08:51,443 - INFO - Downloaded: test.tar.gz
2024-12-06 18:08:51,443 - INFO - Expected md5 is None, skip md5 check for file test.tar.gz.
2024-12-06 18:08:51,443 - INFO - Writing into directory: .. If the results like above, that's expected behavior. Your opinion sounds like |
Regarding file downloading from URL, sometimes we can infer the name from the content deposition. Here is one example snippet: https://github.com/Project-MONAI/VLM/blob/7be688aa457a1806f908eb758f2f3ee816fea017/m3/demo/experts/utils.py#L135 @KumoLiu |
Thank @mingxin-zheng provide information to find file name. |
Signed-off-by: jerome_Hsieh <[email protected]>
Hi @KumoLiu the newest commit I change the logic that if the file path is a directory and not empty, it will automatically capture the name from content deposition, if it doses't have any file name, will use url basename and download the file into the specified directory then gives a warning. |
for more information, see https://pre-commit.ci
Hi @Jerome-Hsieh, thank you for the update. After our discussion in the development meeting, I realized that there is an |
Hi @KumoLiu I see, and what do you think about previous version a9a0171 I commit ? |
Hi @Jerome-Hsieh, a9a0171 looks good. The only update needed is how to extract the file extension from the URL. For example, given a URL like "https://drive.google.com/u/1/uc?id=1KntZge40tAHgyXmHYVqZZ5d2p_4Qr2l5&export=download", you can use |
Signed-off-by: jerome_Hsieh <[email protected]>
@KumoLiu the URL you provide can download but can't extract, I extract manually still fail. |
Signed-off-by: jerome_Hsieh <[email protected]>
Signed-off-by: jerome_Hsieh <[email protected]>
Signed-off-by: jerome_Hsieh <[email protected]>
Signed-off-by: jerome_Hsieh <[email protected]>
@KumoLiu Thanks a lot for your assistance!It looks more cleaner than before! |
/build |
@ericspod do you have any further comments here? Otherwise I will try to merge this one, thanks. |
Fixes #5463
Description
According to issue, the error messages are not very intuitive.
I think maybe we can check if the file name matches the downloaded file’s base name before starting the download.
If it doesn’t match, it will notify user.
Types of changes
./runtests.sh -f -u --net --coverage
../runtests.sh --quick --unittests --disttests
.make html
command in thedocs/
folder.