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

Weakness: duplicated path concatenation in the toolkit/dataset/vot.py #294

Open
fzh0917 opened this issue Feb 16, 2020 · 9 comments
Open
Labels
bug Something isn't working good first issue Good for newcomers high priority small

Comments

@fzh0917
Copy link

fzh0917 commented Feb 16, 2020

As the title describes, there exists two times path concatenations for the member variable named img_names belonged to the VOTVideo class.
One of them occurs at line 18 in toolkit/dataset/video.py , and the other one is at line 53 in toolkit/dataset/vot.py if the variable load_img is set to False.
Here are some examples.

absolute path

>>> a = '/home/username/test_dir'
>>> b = '/home/username/test_dir/test.py'
>>> c = os.path.join(a, b)
>>> print(c)
/home/username/test_dir/test.py # valid path

relative path

>>> a = './test_dir'
>>> b = './test_dir/test.py'
>>> c = os.path.join(a, b)
>>> print(c)
./test_dir/./test_dir/test.py # invalid path

Consequently, if users pass a relative path to the toolkit.datasets.DatasetFactory.create_dataset method, their programs will crash. So, please fix this weakness timely.

@ZhiyuanChen ZhiyuanChen added bug Something isn't working good first issue Good for newcomers high priority small labels Feb 22, 2020
@ZhiyuanChen
Copy link
Collaborator

Thanks for pointing out
You are more than welcome to fix it and submit a pull request

@VishiNehra
Copy link

Hi, I have a fix for this, but somehow don't have permission to open a PR. Are you guys still accepting contributions?

@ZhiyuanChen
Copy link
Collaborator

Hi, I have a fix for this, but somehow don't have permission to open a PR. Are you guys still accepting contributions?

Of course we are accepting contributions, could you please share the error message?

@VishiNehra
Copy link

Yup, when I push my PR I get the following message:
"remote: Permission to STVIR/pysot.git denied to VishiNehra.
fatal: unable to access 'https://github.com/STVIR/pysot/': The requested URL returned error: 403"
Also, when I tried to make manually make a PR on github as a workaround, the "Create Pull Request" button was greyed out for me

@ZhiyuanChen
Copy link
Collaborator

Yup, when I push my PR I get the following message:
"remote: Permission to STVIR/pysot.git denied to VishiNehra.
fatal: unable to access 'https://github.com/STVIR/pysot/': The requested URL returned error: 403"
Also, when I tried to make manually make a PR on github as a workaround, the "Create Pull Request" button was greyed out for me

You should fork this repo first, which should appear as https://github.com/VishiNehra/pysot/, then, you should push your changes to that repo, and finally submit a pull request from your forked repo

VishiNehra pushed a commit to VishiNehra/pysot that referenced this issue Oct 22, 2020
@VishiNehra
Copy link

Oops, totally forgot about forking the repo, thanks!

ZhiyuanChen added a commit that referenced this issue Mar 9, 2021
Fixed handling relative paths #294
@arungrace88
Copy link

Hi,
Is this issue closed or rather it is open?. If open, shall I take it up?. Thanks

@zhaojinjian0000
Copy link

zhaojinjian0000 commented Nov 10, 2021

When I test on UAV123 after "cd experiments/siamrpn_r50_l234_dwxcorr",

x == "group1/001333.jpg"
os.path.abspath(x) == "~/pysot/experiments/siamrpn_r50_l234_dwxcorr/group1/001333.jpg"

However, the converted x should be

"~/pysot/testing_dataset/UAV123/group1/001333.jpg"

In this situation, the code should be

self.img_names = [os.path.join(root, x) for x in img_names]

The "img_names" should be relative path starting from "root".

AdrienGuo pushed a commit to AdrienGuo/pysot that referenced this issue Jul 16, 2022
AdrienGuo pushed a commit to AdrienGuo/pysot that referenced this issue Jul 16, 2022
@ArjunMenon-bit
Copy link

@ZhiyuanChen There is a PR merged for this issue. Is this issue closed? If not, I would like to contribute

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers high priority small
Projects
None yet
Development

No branches or pull requests

6 participants