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
Switched from parameter in can_cast to from_. #126030
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/126030
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit aca228e with merge base d61a81a (): This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Thank you for the PR, I appreciate it. I'm not sure what the most BC friendly way to fix this is, if you want to land this as is, you will need to update the BC script to whitelist this as a BC breaking change. I think I'm willing to give directly landing this as is a try. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is BC-breaking for the C++ API but I think it's fine. Not bc-breaking in python because using from= is a syntax error.
The c++ bc script needs updating though
7dcb439
to
4f50b10
Compare
I updated the BC checking script and will merge when CI is green. I also think the change should be fine, as on the C++ side we can't call by keyword and on the Python side it was a syntax error until now. |
@pytorchbot merge |
Merge failedReason: This PR needs a If not, please add the To add a label, you can comment to pytorchbot, for example For more information, see Details for Dev Infra teamRaised by workflow job |
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
@pytorchbot revert -m 'Sorry for reverting your change but i need to revert it to avoid a diff train conflict with #125995. Please help rebase and I will reland the change' -c weird |
@pytorchbot successfully started a revert job. Check the current status here. |
This reverts commit 06d6bb4. Reverted #126030 on behalf of https://github.com/huydhn due to Sorry for reverting your change but i need to revert it to avoid a diff train conflict with #125995. Please help rebase and I will reland the change ([comment](#126030 (comment)))
@tringwald your PR has been successfully reverted. |
This PR was reopened (likely due to being reverted), so your approval was removed. Please request another review.
4f50b10
to
aca228e
Compare
@huydhn The conflict should be resolved now. Can we reland this again? |
@pytorchbot merge |
This PR needs to be approved by an authorized maintainer before merge. |
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Fixes pytorch#126012. `from` is a reserved keyword in Python, thus we can't make the C++ impl available with `from` as function parameter. This PR changes the name to `from_` and also adjusts the docs. If we want to preserve backwards compatibility, we can leave the C++ name as-is and only fix the docs. However, `torch.can_cast(from_=torch.int, to=torch.int)` won't work then. Pull Request resolved: pytorch#126030 Approved by: https://github.com/albanD
This reverts commit 06d6bb4. Reverted pytorch#126030 on behalf of https://github.com/huydhn due to Sorry for reverting your change but i need to revert it to avoid a diff train conflict with pytorch#125995. Please help rebase and I will reland the change ([comment](pytorch#126030 (comment)))
Fixes pytorch#126012. `from` is a reserved keyword in Python, thus we can't make the C++ impl available with `from` as function parameter. This PR changes the name to `from_` and also adjusts the docs. If we want to preserve backwards compatibility, we can leave the C++ name as-is and only fix the docs. However, `torch.can_cast(from_=torch.int, to=torch.int)` won't work then. Pull Request resolved: pytorch#126030 Approved by: https://github.com/albanD
Fixes #126012.
from
is a reserved keyword in Python, thus we can't make the C++ impl available withfrom
as function parameter. This PR changes the name tofrom_
and also adjusts the docs.If we want to preserve backwards compatibility, we can leave the C++ name as-is and only fix the docs. However,
torch.can_cast(from_=torch.int, to=torch.int)
won't work then.cc @ezyang @gchanan @albanD