Skip to content

Use functools.cached_property #40607

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 10, 2025
Merged

Use functools.cached_property #40607

merged 4 commits into from
Sep 10, 2025

Conversation

cyyever
Copy link
Contributor

@cyyever cyyever commented Sep 2, 2025 •
edited
Loading

What does this PR do?

Use functools.cached_property rather than the shipped one.

@cyyever cyyever changed the title cached_property is avaiable in functools Use functools.cached_property Sep 2, 2025
@cyyever cyyever force-pushed the remove_cached_property branch from df26061 to 487fefe Compare September 2, 2025 04:07
Rocketknight1
Rocketknight1 approved these changes Sep 2, 2025
View reviewed changes
Copy link
Member

@Rocketknight1 Rocketknight1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Yes, this seems like a good PR! We can also remove the cached_property decorator from utils.generic entirely, since it's built-in after 3.8 and our minimum version is Py 3.9. Made one comment but otherwise we should be good to go.

@cyyever
Copy link
Contributor Author

cyyever commented Sep 2, 2025

I am afraid of breaking others code because they may import it.

@cyyever cyyever force-pushed the remove_cached_property branch 3 times, most recently from 860ed3a to 9552693 Compare September 2, 2025 14:56
@Rocketknight1
Copy link
Member

@cyyever yeah, good instincts on not wanting to break remote code models! I think it's okay, though - because cached_property is mostly used in testing code and not in modeling code, I think it's safe to remove it from utils without breaking custom models.

@cyyever
Copy link
Contributor Author

cyyever commented Sep 3, 2025

@Rocketknight1 Ok, let's remove it.

@cyyever
cached_property is avaiable in functools
55de9fe 
Signed-off-by: cyy <cyyever@outlook.com>
@cyyever cyyever force-pushed the remove_cached_property branch from b280676 to d897c9d Compare September 3, 2025 13:37
@cyyever
Remove cached_property
46ac768 
Signed-off-by: cyy <cyyever@outlook.com>
@cyyever cyyever force-pushed the remove_cached_property branch from d897c9d to 46ac768 Compare September 3, 2025 13:39
@cyyever
Copy link
Contributor Author

cyyever commented Sep 8, 2025

@Rocketknight1 Ping

Rocketknight1
Rocketknight1 approved these changes Sep 8, 2025
View reviewed changes
Copy link
Member

@Rocketknight1 Rocketknight1 left a comment

Choose a reason for hiding this comment

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

Yep, looks good to me now!

@Rocketknight1 Rocketknight1 enabled auto-merge (squash) September 8, 2025 15:59
auto-merge was automatically disabled September 8, 2025 16:15

Head branch was pushed to by a user without write access

@cyyever
Copy link
Contributor Author

cyyever commented Sep 8, 2025

@Rocketknight1 It is removed from doc, otherwise doc can't build.

Rocketknight1
Rocketknight1 reviewed Sep 9, 2025
View reviewed changes
@cyyever cyyever force-pushed the remove_cached_property branch from ef9f620 to 8bd1ceb Compare September 9, 2025 12:40
@cyyever
Fix docs
cf521fe 
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
@cyyever cyyever force-pushed the remove_cached_property branch from 8bd1ceb to cf521fe Compare September 9, 2025 12:41
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 9, 2025

[For maintainers] Suggested jobs to run (before merge)

run-slow: audio_spectrogram_transformer, bark, bart, beit, bert_generation, big_bird, bit, blenderbot, blenderbot_small, bridgetower, byt5, canine, codegen, conditional_detr, convnext, convnextv2

@Rocketknight1
Copy link
Member

LGTM now!

@Rocketknight1 Rocketknight1 enabled auto-merge (squash) September 10, 2025 12:06
@Rocketknight1 Rocketknight1 merged commit 7401cfa into huggingface:main Sep 10, 2025
23 checks passed
@HuggingFaceDocBuilderDev
Copy link

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.

@cyyever cyyever deleted the remove_cached_property branch September 10, 2025 12:17
vijayabhaskar-ev pushed a commit to vijayabhaskar-ev/transformers that referenced this pull request Oct 2, 2025
@cyyever @vijayabhaskar-ev
Use functools.cached_property (huggingface#40607)
4ecbc04 
* cached_property is avaiable in functools

Signed-off-by: cyy <cyyever@outlook.com>

* Remove cached_property

Signed-off-by: cyy <cyyever@outlook.com>

* Fix docs

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

---------

Signed-off-by: cyy <cyyever@outlook.com>
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
yuchenxie4645 pushed a commit to yuchenxie4645/transformers that referenced this pull request Oct 4, 2025
@cyyever @yuchenxie4645
Use functools.cached_property (huggingface#40607)
44039c7 
* cached_property is avaiable in functools

Signed-off-by: cyy <cyyever@outlook.com>

* Remove cached_property

Signed-off-by: cyy <cyyever@outlook.com>

* Fix docs

Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>

---------

Signed-off-by: cyy <cyyever@outlook.com>
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rocketknight1 Rocketknight1 Rocketknight1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cyyever @Rocketknight1 @HuggingFaceDocBuilderDev