Skip to content

Remove .JPEG & .PNG from extension list as .jpeg & .png are already present. #142

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

Closed
wants to merge 1 commit into from

Conversation

GirinChutia
Copy link

@GirinChutia GirinChutia commented Mar 29, 2023

BUG
When a image folder has .jpeg or .png extension images. Imagelab loads that image twice as shown below,

image

Its happening because in src/cleanvision/utils/utils.py the extension lists have both '.JPEG' and '.jpeg', same for .png as well.

image

When loaded in the downstream code (by looping the extension list) using glob module to load the images, if a image has '.JPEG' extension, it load once for the '.JPEG' and once again for '.jpeg' because glob module treats both '.JPEG' and '.jpeg' as same image i.e, it doesn't matters if the extension is all caps or not.

Solution
Just keep '.jpeg' and '.png' in the list. It will handle even if the extension of the image is in all caps.

@CLAassistant
Copy link

CLAassistant commented Mar 29, 2023

CLA assistant check
All committers have signed the CLA.

@jwmueller jwmueller requested a review from sanjanag March 30, 2023 01:30
@jwmueller
Copy link
Member

Thank you for pointing out this bug and contributing a bugfix! However the true root cause is the fact that glob is case-sensitive on Mac/Linux, but is case-insensitive on Windows.

https://jdhao.github.io/2019/06/24/python_glob_case_sensitivity/

We didn't do enough testing on Windows, but will start including Windows in our CI tests asap!
That said, this fix will not work since it will break things on Mac/Linux where glob is case-sensitive. Feel free to propose alternative fixes if you have thoughts, otherwise our team will be pushing a fix shortly.

@jwmueller
Copy link
Member

Closing this PR in favor of: #143

Note the bug was just on Windows, so thanks a ton for pointing it out. Feel free to make other PRs if you see more opportunities for improvement!

@jwmueller jwmueller closed this Mar 31, 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.

3 participants