Comprehensive type checking for from_pretrained kwargs#10758
Comprehensive type checking for from_pretrained kwargs#10758hlky merged 14 commits intohuggingface:mainfrom
from_pretrained kwargs#10758Conversation
|
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. |
hlky
left a comment
There was a problem hiding this comment.
Thanks @guiyrt, nice work. Could you take a look through pipeline test output searching for Expected types for (GitHub's built in search works best)
There are some easy cases that we could fix like
Expected types for feature_extractor: (<class 'transformers.models.clip.image_processing_clip.CLIPImageProcessor'>,), got <class 'transformers.models.clip.feature_extraction_clip.CLIPFeatureExtractor'>.
For that we could do global find+replace on feature_extractor: CLIPImageProcessor -> feature_extractor: CLIPFeatureExtractor.
and some that need investigating
Expected types for unet: (<class 'inspect._empty'>,), got <class 'diffusers.models.unets.unet_2d.UNet2DModel'>.
Type correctness is not strictly enforced so some warnings are expected but we should make a best effort to reduce the number of new warnings that we're introducing. If we find a particular component to be a problem we can skip it like scheduler.
|
Failing tests appear unrelated, will re-run later. |
|
@hlky Findings from looking through the test logs TL;DR 1. Using XYZFast
|
|
Found another warning related to custom pipelines, this time on "hf-internal-testing/diffusers-dummy-pipeline". The fix is having the correct type hinting there. 4 occurrences diffusers/tests/pipelines/test_pipelines.py Lines 1043 to 1053 in 067eab1 |
|
I opened PRs on the hf-internal-testing repos with warnings: The last one is regarding arguments with no annotations, which might be common on custom pipelines? To keep warnings relevant, it might be a good idea to skip type checking if there is no type annotated for a given argument. |
|
This to-do includes:
|
|
@hlky can we move the functions |
|
@yiyixuxu WDYT about |
I updated the docs. But if I got it right, Lumina v1 uses |
|
Tests failed due to network issues I think. I noticed yesterday very slow download speeds from the hub, anything you are aware? |
|
Gentle ping @yiyixuxu |
|
thanks for the PR @guiyrt @hlky |
|
Failing tests are unrelated. Thanks @guiyrt |
What does this PR do?
Changes
List[ControlNetModel]instead oflist)To-do
is_valid_typeandget_detailed_typebe placed?According to new functions location, add simple tests for type checking.These changes are proposed based on testing for #10747.
Example warning when providing
controlnetasList[ControlNetUnionModel]forStableDiffusionXLControlNetPipeline, whereList[ControlNetModel]is expected:Code for warning replication
Before submitting
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@hlky