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

Replace TypedStorage._storage with TypedStorage.untyped() #221

Closed
wants to merge 1 commit into from

Conversation

kurtamohler
Copy link
Contributor

@kurtamohler kurtamohler commented Oct 20, 2022

As part of my PR pytorch/pytorch#85303, I'm renaming TypedStorage._storage to TypedStorage._untyped_storage for better clarity, and since MultiPy depends on the old name, a CI job is failing on my PR. There is a public function TypedStorage.untyped() which is probably better to use in this case, and doing so will fix the CI failure for me

cc @ezyang

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 20, 2022
@ezyang ezyang requested a review from d4l3k October 21, 2022 01:31
@PaliC PaliC self-requested a review October 24, 2022 20:25
@kurtamohler
Copy link
Contributor Author

@PaliC, the failing lint test seems to be unrelated to my change, right?

From the log:

2022-10-24T20:26:25.0302473Z Checking multipy/utils/_deploy.py
2022-10-24T20:26:26.0979083Z multipy/utils/_deploy.py:20:1: E722 do not use bare 'except'
2022-10-24T20:26:26.0980114Z multipy/utils/_deploy.py:90:13: E722 do not use bare 'except'
2022-10-24T20:26:26.1395776Z One of the linters returned an error. See above output.
2022-10-24T20:26:26.1404280Z ##[error]Process completed with exit code 1.

I did change that file, but I didn't touch those lines

@ezyang
Copy link

ezyang commented Nov 5, 2022

Cc @d4l3k

Copy link
Contributor

@PaliC PaliC left a comment

Choose a reason for hiding this comment

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

lgtm, sorry forgot to approve externally.

The failing lint tests are not your fault.

Also this repo does not a github source of truth yet / does not have a diff train, so please import the changes internally.

@ezyang
Copy link

ezyang commented Nov 7, 2022

@kurtamohler is not an FB employee, one of you will have to do it lol

@facebook-github-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants