search

roboflow / notebooks

Examples and tutorials on using SOTA computer vision models and techniques. Learn everything from old-school ResNet, through YOLO and object-detection transformers like DETR, to the latest models like Grounding DINO and SAM.
https://roboflow.com/models
4.78k stars 744 forks source link

Call raise_for_status after retrieving a URL with requests #176

Closed jkitching closed 9 months ago

jkitching commented 11 months ago

Description

Sometimes, image files may fail to download due to HTTP errors such as "429 Too Many Requests". As a result of the image file on disk being empty, errors may occur later on in the notebook, which can look somewhat misleading:

cv2.error: OpenCV(4.7.0) /io/opencv/modules/imgcodecs/src/loadsave.cpp:798: error: (-215:Assertion failed) !buf.empty() in function 'imdecode_'

Add a call to raise_for_status() on the returned response object from the requests library, which at the very least notifies the user that something has gone wrong.

(Alternately, perhaps consider relocating the images from imgur to some other CDN, or even commit them directly to this repository.)

Also, fix some indentation, spacing, and variable naming inconsistencies.

Type of change

Bug fix (non-breaking change which fixes an issue)

How has this change been tested, please provide a testcase or example of how you tested the change?

Manually tested running the notebooks, ensuring the 429 error is displayed to the user.

Any specific deployment considerations

None.

Docs

No documentation was updated.

CLAassistant commented 11 months ago

CLA assistant check
All committers have signed the CLA.

SkalskiP commented 11 months ago

Hi, @jkitching 👋🏻! I was away for a few days. Awesome 🔥! Thanks for that contribution. Could you sign the CLA so I could merge it into the master? 🙏🏻

jkitching commented 11 months ago

Hi @SkalskiP, thanks for your response! I've signed the CLA.

Is the same CLA also valid for this other pull request? https://github.com/roboflow/video-inference/pull/4

capjamesg commented 9 months ago

@jkitching The video-inference repository is mostly obsolete now. Piotr made a newer version that uses Roboflow Inference, our open source inference server. I will chat with our team to see if we should archive video-inference.