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

Fix ModelHubMixin: pass config when __init__ accepts **kwargs #2058

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Feb 28, 2024

Fix a breaking change introduced in #2001.

Since #2001, we fetch the config.json file in ModelHubMixin and parse the config as a dict or dataclass depending on the expected param in __init__. If config is not expected, we don't pass it to _from_pretrained (otherwise it'll brake when instantiating the class). This PR fixes this behavior by sending the config value also when __init__ accepts **kwargs, which is the default when nothing is defined.

cc @mfuntowicz who noticed the breaking change

Once this PR is approved and merged, I'll make a hot-fix release (0.21.3).

@HuggingFaceDocBuilderDev

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.

Copy link

codecov bot commented Feb 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.72%. Comparing base (e7f243c) to head (87ad4ee).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2058      +/-   ##
==========================================
+ Coverage   80.70%   80.72%   +0.02%     
==========================================
  Files          71       71              
  Lines        8519     8521       +2     
==========================================
+ Hits         6875     6879       +4     
+ Misses       1644     1642       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Makes sense! Thanks for fixing it!

@Wauplin
Copy link
Contributor Author

Wauplin commented Feb 29, 2024

Thanks for the quick review @LysandreJik! I'm merging now and will proceed with 0.21.3 release.

@Wauplin Wauplin merged commit 014e8b4 into main Feb 29, 2024
16 checks passed
@Wauplin Wauplin deleted the fix-hub-mixin-if-kwargs-in-init branch February 29, 2024 08:20
Wauplin added a commit that referenced this pull request Feb 29, 2024
)

* Fix ModelHubMixin: pass config when __init__ accepts **kwargs

* comment
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

3 participants