Skip to content

Replace old download scripts - #239

Merged
PINTO0309 merged 7 commits into
PINTO0309:mainfrom
ibaiGorordo:main
Mar 26, 2022
Merged

PINTO0309 merged 7 commits into
PINTO0309:mainfrom
ibaiGorordo:main

Conversation

@ibaiGorordo

Copy link
Copy Markdown
Collaborator

This pull requests replaces the old download links with the new pattern (sample: db4e3af).

The replacement has been done using the following script: https://gist.github.com/ibaiGorordo/69988fbf83a045e6c58701aa03eb423f

I have not tested the changes, so there might be issues with some of the conversions.

@PINTO0309

Copy link
Copy Markdown
Owner

Too wonderful. Other engineers are starting to work on converting the target model, so I will temporarily hold off on this pull request, although I very much appreciate it. However, it appears to be a very effective means of filling in gaps in other engineers' modifications. ðŸ’Ŋ

https://github.com/PINTO0309/PINTO_model_zoo/issues
image

@ibaiGorordo

Copy link
Copy Markdown
Collaborator Author

I realized the are mistakes in scripts with two downloads. I need to modify the script to search for the curl line, instead of hard code define the lines.

To avoid the chaos, you can ignore this PR and convert the other models in your local copy.

@PINTO0309 PINTO0309 added the Awesome Script Very good improvement suggestions label Mar 26, 2022
@PINTO0309 PINTO0309 added the pending Temporary hold due to conflict with other work label Mar 26, 2022
@ibaiGorordo

Copy link
Copy Markdown
Collaborator Author

I updated the conversion script for multiple downloads and I think the rest are fine, but I might have misses some files with a different pattern.

@PINTO0309

Copy link
Copy Markdown
Owner

Thanks!

@PINTO0309

Copy link
Copy Markdown
Owner

@ibaiGorordo Thank you for your time, but can you resolve the conflict? Once the conflict is resolved I would like to merge your proposals in bulk.

@ibaiGorordo

Copy link
Copy Markdown
Collaborator Author

I think it should be fine now. I modified those files manually, but if necessary I think it should be possible to add the rm cookie for the rest of the scripts

@PINTO0309

Copy link
Copy Markdown
Owner

Thank you! I will reflect the changes once in this state.

@PINTO0309
PINTO0309 merged commit b02c44f into PINTO0309:main Mar 26, 2022
@PINTO0309 PINTO0309 removed the pending Temporary hold due to conflict with other work label Mar 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Awesome Script Very good improvement suggestions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants