Remove requests code#42845
Conversation
|
Hello, I got some issues with the current port to httpx. One way to do it would be for example: This also concerns some other portions of code (like |
|
Agree with @guibruand 's suggestion here! It's best to use |
Got it. I'll try using that and let you know! |
|
Thank you very much! 🤗 |
|
Hey @Wauplin , so I've used Thanks |
Wauplin
left a comment
There was a problem hiding this comment.
Hi @omkar-334 , thanks for the changes and sorry for the delay before reviewing. I'm very sorry about my previous comment regarding get_session. I should have more carefully double-checked the changes and context before answering. It turns out most (if not all) changes made in the PR would better benefit from using httpx directly for simplicity rather than huggingface_hub.get_session.
While get_session is better suited to make requests to the Hub (it has HF-specific features), plain httpx is better suited for generic cases that are not HF-related. Also, let's use httpx in all 1-file scripts and utilities (since they are mostly meant for devs/maintainers rather than end users).
Sorry again about this change of direction 🙏
| import requests | ||
| import tensorflow as tf | ||
| import torch | ||
| from huggingface_hub import get_session |
There was a problem hiding this comment.
Same as above (and sorry not being explicit before). For single-file scripts I think it's best to keep it as lean as possible, i.e. use httpx directly as replacement of requests.
get_session is useful when calling the Hub. It adds features to handle request ids and manage offline mode. When in doubt you can use get_session but otherwise keep httpx for light calls to "generic endpoints" (like here, loading an image from the internet).
|
hey @Wauplin , went through your comments, understood the changes i need to make. Thanks for clarifying this.
Will make the changes and push soon... |
|
hey @Wauplin , I've made the changes.
Thanks! |
|
@bot /style |
|
Style fix is beginning .... View the workflow run here. |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
Hi @omkar-334 sorry getting back to it only now. A few notes:
- [0.0000, 0.0000, 0.0000, 0.0000, 0.5300, 0.0000, 0.0000, 0.0000, 0.0000, 1.1221]
+ [
+ 0.0000,
+ 0.0000,
+ 0.0000,
+ 0.0000,
+ 0.5300,
+ 0.0000,
+ 0.0000,
+ 0.0000,
+ 0.0000,
+ 1.1221,
+ ]Can you revert them? The PR is already huge like this so I'd like to keep it "as small as possible". Same for the - (f"encoder.deit.blocks.{i}.norm2.weight", f"encoder.encoder.layer.{i}.layernorm_after.weight")
+ (
+ f"encoder.deit.blocks.{i}.norm2.weight",
+ f"encoder.encoder.layer.{i}.layernorm_after.weight",
+ )etc.
- with open(filename, "w") as f:
- f.write("\n".join(new_file_lines))
+ Path(filename).write_text("\n".join(new_file_lines))While I agree with this codestyle, I do think the changes should be made separately
Let's keep them as they are before this PR.
Let's change them to Thanks in advance! |
|
hey @Wauplin , i've reverted the formatting changes and updated the docstrings. I think it's good to go now. Thanks! |
…ers into remove-requests-code
Wauplin
left a comment
There was a problem hiding this comment.
Thanks for this last iteration @omkar-334! I've made a few changes to fix the last remaining issues and reviewed all files manually. Took some time but definitely a good thing to clean up!
Let's wait for a last approval from an official @huggingface/transformers-core-maintainers 🤗
|
failing test seems unrelated I cannot reproduce it locally 😕 |
|
[For maintainers] Suggested jobs to run (before merge) run-slow: aimv2, align, altclip, aria, beit, bit, blip, blip_2, bridgetower, chameleon, chinese_clip, clip, clipseg |
Thanks for the careful review and cleanup! @Wauplin .... my auto-format-on-save and search-and-replace on VS Code messed up things, but looks good in the end! |
|
Thanks for your work! I'll be shipped as part of the v5 release (coming very soon) |
* setup, workflow and utils files * src core files * use get_session instead of httpx * fix code quality - remove unused imports * utils and prevous-http-used and cli * style changes * use httpx for non-HF images * get_session for HF images * change docstrings * revert formatting - 1 * revert formatting - 2 * docstrings fixes * revert formatting - 3 * docstring fixes * fix failing test * Update src/transformers/pipelines/image_to_image.py * Update src/transformers/testing_utils.py * fix modular generation check * fix consistency * Apply suggestions from code review * Update src/transformers/models/idefics2/modeling_idefics2.py * Update src/transformers/models/idefics3/modeling_idefics3.py --------- Co-authored-by: Lucain <lucainp@gmail.com>
What does this PR do?
Removes
requestscode from setup, src and utils files.Fixes #42817 partially
make fixuppasses with no errorscc @CoderTCY @Wauplin @Rocketknight1
notes -
requestsfrom setup/workflow files as well, you might want to check the first commit.