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

typing: Use Self from typing_extensions #2494

Closed
wants to merge 6 commits into from

Conversation

eddiebergman
Copy link
Contributor

@eddiebergman eddiebergman commented Aug 29, 2024

Motivation

Why wait till 3.11 :) The dependancy of typing_extensions is already present in torch and is a core python development library with the same license as python itself.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

I believe this only needs to pass the lint checks. However as it adds a new dependancy (which should be installed already with torch), there could be issues with action runners if installation of dependencies is done in a non-standard manner. However it seems like it should be fine.

Related PRs

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Aug 29, 2024
@facebook-github-bot
Copy link
Contributor

@sdaulton has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link

codecov bot commented Aug 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.98%. Comparing base (d52205a) to head (d6a5c03).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2494   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         193      193           
  Lines       16947    16947           
=======================================
  Hits        16945    16945           
  Misses          2        2           

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

@sdaulton
Copy link
Contributor

Looks good to me. Could you put the new import in alphabetical order to resolve the formatting failure? Thanks!

@eddiebergman
Copy link
Contributor Author

Hi @sdaulton,

Sorry, I was on vacation and then there was a conference I was attending. I've fixed the formatting (I believe) and also merged with the most current main which git marked as a conflict

@facebook-github-bot
Copy link
Contributor

@Balandat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@eddiebergman
Copy link
Contributor Author

Noticed one more formatting issue from github actions, fixed and pushed

@facebook-github-bot
Copy link
Contributor

@Balandat merged this pull request in ec2ad88.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Style] Use typing_extensions.Self pre-3.11 Python
3 participants