[DataPipe] Fixing map function signature validation
#84279
Closed
+8
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ghstack-poisoned]
🔗 Helpful links
✅ No Failures (0 Pending)As of commit 161eca9 (more details on the Dr. CI page): Expand to see more💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
NivekT
added a commit
that referenced
this pull request
Aug 30, 2022
ghstack-source-id: 5742ef47c4293862545fe852233315a224e8814e Pull Request resolved: #84279
NivekT
requested review from
VitalyFedyunin,
ejguan and
pmeier
and removed request for
VitalyFedyunin and
ejguan
NivekT
changed the title
map function sigature validation
pmeier
approved these changes
Aug 30, 2022
NivekT
changed the title
map function sigature validationmap function signature validation
|
Build failure unrelated (the |
|
@pytorchbot rebase |
|
@pytorchbot successfully started a rebase job. Check the current status here |
As pmeier [points out](#80267 (comment)), #80267 introduces a bug where an exception is thrown when a built-in function (or a function implemented in C) is used with `.map` because `inspect.signature(fn)` cannot find the function's signature. This PR skips over a function when its signature cannot be found. I believe this case is rare, and if the `fn` is truly incompatible with the usage of `input_col`/`output_col`, an exception will be raised at run time such that users will be able to examine what is wrong. [ghstack-poisoned]
|
Successfully rebased |
|
@pytorchbot merge |
Merge failedReason: Matched rule Core Maintainers, but PR #84279 was not reviewed yet by any of: ezyang, gchanan, soumith, dzhulgakov Details for Dev Infra teamRaised by workflow job |
janeyx99
approved these changes
Aug 31, 2022
|
@pytorchbot merge |
|
Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea. |
|
@pytorchbot successfully started a merge job. Check the current status here. |
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 31, 2022
… to allow more appropriate messages to be raised first" Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 31, 2022
…ppropriate messages to be raised first" Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." [ghstack-poisoned]
Merge failedReason: View failures on hud. Refusing to merge as mandatory check(s) pull failed for rule superuser. Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot rebase |
|
@pytorchbot successfully started a rebase job. Check the current status here |
As pmeier [points out](#80267 (comment)), #80267 introduces a bug where an exception is thrown when a built-in function (or a function implemented in C) is used with `.map` because `inspect.signature(fn)` cannot find the function's signature. This PR skips over a function when its signature cannot be found. I believe this case is rare, and if the `fn` is truly incompatible with the usage of `input_col`/`output_col`, an exception will be raised at run time such that users will be able to examine what is wrong. [ghstack-poisoned]
|
Successfully rebased |
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 31, 2022
ghstack-source-id: 7d75fcc597d83e0d3524e915116cde9205947326 Pull Request resolved: #84279
|
@pytorchbot merge |
|
@pytorchbot successfully started a merge job. Check the current status here. |
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 31, 2022
… to allow more appropriate messages to be raised first" Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 31, 2022
…ppropriate messages to be raised first" Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 31, 2022
… to allow more appropriate messages to be raised first" Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 31, 2022
…ppropriate messages to be raised first" Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Sep 1, 2022
… to allow more appropriate messages to be raised first" Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Sep 1, 2022
…ppropriate messages to be raised first" Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." [ghstack-poisoned]
pytorchmergebot
pushed a commit
that referenced
this pull request
Sep 1, 2022
…messages to be raised first (#84359) Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) @janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." Pull Request resolved: #84359 Approved by: https://github.com/janeyx99, https://github.com/ZainRizvi, https://github.com/malfet
facebook-github-bot
pushed a commit
that referenced
this pull request
Sep 2, 2022
Summary: As pmeier [points out](#80267 (comment)), #80267 introduces a bug where an exception is thrown when a built-in function (or a function implemented in C) is used with `.map` because `inspect.signature(fn)` cannot find the function's signature. This PR skips over a function when its signature cannot be found. I believe this case is rare, and if the `fn` is truly incompatible with the usage of `input_col`/`output_col`, an exception will be raised at run time such that users will be able to examine what is wrong. Pull Request resolved: #84279 Approved by: https://github.com/pmeier, https://github.com/janeyx99 Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/cfb9d0d23314fd28be118b6ca280ded55364e71c Reviewed By: mehtanirav Differential Revision: D39213537 Pulled By: NivekT fbshipit-source-id: 58d176edb3877b0ed0ee74ef4b3a3e61950f7111
facebook-github-bot
pushed a commit
that referenced
this pull request
Sep 2, 2022
…messages to be raised first (#84359) (#84359) Summary: Changing the ordering in merge rules to allow more appropriate messages to be raised first. Context: [#84279](#84279 (comment)) janeyx99: "Approving to unblock, but modifying the merge rules to move the Core maintainers rule to last would be a good idea." Pull Request resolved: #84359 Approved by: https://github.com/janeyx99, https://github.com/ZainRizvi, https://github.com/malfet Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/d648375f13b4a4efd4cd35247098679fce5d4bcd Reviewed By: mehtanirav Differential Revision: D39214439 Pulled By: NivekT fbshipit-source-id: 0fbff5b4aacb9365bdced9d7bbe28bd7c7815a55
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack (oldest at bottom):
mapfunction signature validation #84279As @pmeier points out, #80267 introduces a bug where an exception is thrown when a built-in function (or a function implemented in C) is used with
.mapbecauseinspect.signature(fn)cannot find the function's signature.This PR skips over a function when its signature cannot be found. I believe this case is rare, and if the
fnis truly incompatible with the usage ofinput_col/output_col, an exception will be raised at run time such that users will be able to examine what is wrong.