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

Windows Python 3.7 .isfile() fix #9879

Merged
merged 12 commits into from
Oct 24, 2022
Merged

Conversation

SSTato
Copy link
Contributor

@SSTato SSTato commented Oct 21, 2022

Signed-off-by: SSTato 1210546396@qq.com

πŸ› οΈ PR Summary

Made with ❀️ by Ultralytics Actions

🌟 Summary

Improved file existence checks in Ultralytics dataloaders and utility functions.

πŸ“Š Key Changes

  • Modified the method of checking if a file exists from using Path.is_file() to os.path.isfile().
  • Clarified and standardized the file existence validation across different parts of the code.

🎯 Purpose & Impact

  • Consistency: Standardizing file checks makes the codebase more uniform and easier to maintain.
  • Compatibility: The os module's file check methods are widely accepted, ensuring better compatibility across different operating systems.
  • Reliability: These changes could increase the reliability of file handling within the toolkit, which benefits users in smoother data loading and processing.

Signed-off-by: SSTato <1210546396@qq.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ‘‹ Hello @SSTato, thank you for submitting a YOLOv5 πŸš€ PR! To allow your work to be integrated as seamlessly as possible, we advise you to:

  • βœ… Verify your PR is up-to-date with ultralytics/yolov5 master branch. If your PR is behind you can update your code by clicking the 'Update branch' button or by running git pull and git merge master locally.
  • βœ… Verify all YOLOv5 Continuous Integration (CI) checks are passing.
  • βœ… Reduce changes to the absolute minimum required for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." β€” Bruce Lee

@SSTato
Copy link
Contributor Author

SSTato commented Oct 21, 2022

Resolve Issue #9866

@glenn-jocher
Copy link
Member

@SSTato I added Windows URL download test to CI in

check_requirements(exclude=('tensorboard', 'thop'))
and observed no problems in Windows.

See glenn-jocher#2

Screenshot 2022-10-21 at 21 29 11

@SSTato
Copy link
Contributor Author

SSTato commented Oct 22, 2022

I replaced my computer with a new one and downloaded the latest yolov5-master. He had no problem checking the pictures.
1

However, when I check the URL, I still make mistakes. The problem is still the line of code.
2

It does have problems with Windows 10, and I've tried it many times.
My new computer configuration is:
Windows 10 Python-3.7.10 torch-1.8.1+cu111 CUDA:0 (GeForce RTX 3060 Ti, 8192MiB)

@SSTato
Copy link
Contributor Author

SSTato commented Oct 22, 2022

The problem is that the code 'Path(sources).is_file()' cannot run correctly on my computer. Replacing it with 'os.path.isfile(sources)' can solve this problem.
Other people have encountered the same problem and contacted me via email to solve it.

Signed-off-by: SSTato <1210546396@qq.com>
@glenn-jocher
Copy link
Member

@SSTato I wonder if it's a Python 3.7 issue with Windows. I'll update the CI to 3.7 to see what happens.

@SSTato
Copy link
Contributor Author

SSTato commented Oct 22, 2022

@SSTato I wonder if it's a Python 3.7 issue with Windows. I'll update the CI to 3.7 to see what happens.

Python 3.8 has the same problem

@glenn-jocher
Copy link
Member

Ok yes I see that 3.7 Pathlib usage might not allow for .is_file()

I wonder if we can replace it with .exists()

Screenshot 2022-10-22 at 15 47 28

@glenn-jocher glenn-jocher changed the title Update dataloaders.py Windows Python 3.7 .isfile() fix Oct 22, 2022
Signed-off-by: Glenn Jocher <glenn.jocher@ultralytics.com>
Signed-off-by: Glenn Jocher <glenn.jocher@ultralytics.com>
Signed-off-by: Glenn Jocher <glenn.jocher@ultralytics.com>
@glenn-jocher
Copy link
Member

glenn-jocher commented Oct 22, 2022

@SSTato ok I've updated based on fixes in glenn-jocher#2 that pass Windows 3.7, 3.8 CI. Can you try the PR now and let me know if this resolves your issue?

Signed-off-by: SSTato <1210546396@qq.com>
Signed-off-by: SSTato <1210546396@qq.com>
Signed-off-by: SSTato <1210546396@qq.com>
Signed-off-by: SSTato <1210546396@qq.com>
Signed-off-by: SSTato <1210546396@qq.com>
Signed-off-by: SSTato <1210546396@qq.com>
Signed-off-by: SSTato <1210546396@qq.com>
@SSTato
Copy link
Contributor Author

SSTato commented Oct 24, 2022

@SSTato ok I've updated based on fixes in glenn-jocher#2 that pass Windows 3.7, 3.8 CI. Can you try the PR now and let me know if this resolves your issue?

I tested glenn-jocher#2, which can be used in python 3.7. For ' python detect py --source https://ultralytics.com/images/zidane.jpg 'No problem
11

but when I' python detect py --source rtsp://admin:abc12345 @192.168.1.71/Streaming/Channels/1 ', it still has errors, and' dataloaders. py 'needs to be modified. After that, it has no errors.
22

@SSTato
Copy link
Contributor Author

SSTato commented Oct 24, 2022

I can correctly run the code before the change on python 3.9, so it can be determined that the problem is the python version. There is no problem when the yolov5-master is above python 3.9, but there are problems I raised with python 3.7 and 3.8.
22

@glenn-jocher glenn-jocher merged commit fba61e5 into ultralytics:master Oct 24, 2022
@glenn-jocher
Copy link
Member

@SSTato PR is merged. Thank you for your contributions to YOLOv5 πŸš€ and Vision AI ⭐

NagatoYuki0943 added a commit to NagatoYuki0943/yolov5-ultralytics that referenced this pull request Oct 25, 2022
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