-
Notifications
You must be signed in to change notification settings - Fork 940
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
No input mapping fix for dag flows #3377
Conversation
promptflow SDK CLI Azure E2E Test Result users/singankit/dag_flow_load_flow_bug 4 files 4 suites 1m 31s ⏱️ For more details on these errors, see this check. Results for commit bd9b411. |
promptflow-core test result0 tests 0 ✅ 0s ⏱️ Results for commit bd9b411. |
SDK CLI Global Config Test Result users/singankit/dag_flow_load_flow_bug6 tests 6 ✅ 59s ⏱️ Results for commit bd9b411. |
Executor Unit Test Result users/singankit/dag_flow_load_flow_bug797 tests 796 ✅ 3m 45s ⏱️ For more details on these failures, see this check. Results for commit bd9b411. |
Executor E2E Test Result users/singankit/dag_flow_load_flow_bug246 tests 240 ✅ 5m 1s ⏱️ Results for commit bd9b411. |
SDK CLI Test Result users/singankit/dag_flow_load_flow_bug 4 files 4 suites 1h 4m 44s ⏱️ For more details on these failures, see this check. Results for commit bd9b411. |
@@ -86,6 +86,11 @@ def apply_inputs_mapping( | |||
), | |||
invalid_relations=invalid_relations, | |||
) | |||
|
|||
# if no input mapping is provided, return the original inputs which is data | |||
if not result: |
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.
add more PR description on what's the case fixed in this PR?
@@ -86,6 +86,11 @@ def apply_inputs_mapping( | |||
), | |||
invalid_relations=invalid_relations, | |||
) | |||
|
|||
# if no input mapping is provided, return the original inputs which is data |
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.
I think it is not a good idea to add such fallback logic in the "apply_inputs_mapping" function, it should know nothing about the special key "data". If we want to do something special for "data", we'd better handle it outside the utils, where the specific scenario uses it.
Hi, thank you for your interest in helping to improve the prompt flow experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. |
This is under discussion |
Hi, thank you for your interest in helping to improve the prompt flow experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. |
Hi, thank you for your contribution. Since there has not been recent engagement, we are going to close this out. Feel free to reopen if you'd like to continue working on these changes. Please be sure to remove the |
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
All Promptflow Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines