Skip to content
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

[Fix] decode URL encoded string in config path #192

Closed
wants to merge 1 commit into from

Conversation

kbumsik
Copy link

@kbumsik kbumsik commented Jan 9, 2023

Motivation

Some packages has config paths which is url encoded (e.g. %2B for +) https://github.com/open-mmlab/mmdetection/blob/db85fd12afce2fe89d1c7b870874c02a90018a16/configs/gn%2Bws/metafile.yml#L17

Currently downloading those models trrigers "is not found" error because mim does not support url encoded path.

Modification

While I am not sure if this should be fixed in mim or in metafile.yml in the package, I firstly attempt to fix in mim package.

BC-breaking (Optional)

Use cases (Optional)

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  3. If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMCls.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

Some packages has config paths which is url encoded (e.g. %2B for +)
https://github.com/open-mmlab/mmdetection/blob/db85fd12afce2fe89d1c7b870874c02a90018a16/configs/gn%2Bws/metafile.yml#L17

Currently downloading those models trrigers "is not found" error
because mim does not support url encoded path.

While I am not sure if this should be fixed in mim or in metafile.yml in
the package, I firstly attempt to fix in mim package.
@ice-tong
Copy link
Collaborator

Hi @kbumsik, thanks for your kind contribution!

The Config field in Metafile.yml actually should be a file path.

The URL encode in Config path is not a normal behavior, and we will check with mmdetection.

Thanks again!

@kbumsik
Copy link
Author

kbumsik commented Jan 13, 2023

I see, then do you prefer closing this?

I actually fixed all of metafile.yml in mmdetection, including url encoded file paths: open-mmlab/mmdetection#9627

@ice-tong
Copy link
Collaborator

@kbumsik Sorry for the late reply~

I prefer to close this PR, and feel free to reopen it if you have any problems. ^_^

@ice-tong ice-tong closed this Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants