Skip to content

Conversation

SSTato
Copy link
Contributor

@SSTato SSTato commented Oct 21, 2022

Signed-off-by: SSTato [email protected]

πŸ› οΈ 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.

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 #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 <[email protected]>
@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 <[email protected]>
Signed-off-by: Glenn Jocher <[email protected]>
Signed-off-by: Glenn Jocher <[email protected]>
@glenn-jocher
Copy link
Member

glenn-jocher commented Oct 22, 2022

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

@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 #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 ⭐

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.

2 participants