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

PowerBI major refinement in working of tool and tweaks in the rest #5090

Merged
merged 7 commits into from
May 22, 2023

Conversation

eavanvalkenburg
Copy link
Contributor

@eavanvalkenburg eavanvalkenburg commented May 22, 2023

PowerBI major refinement in working of tool and tweaks in the rest

I've gained some experience with more complex sets and the earlier implementation had too many tries by the agent to create DAX, so refactored the code to run the LLM to create dax based on a question and then immediately run the same against the dataset, with retries and a prompt that includes the error for the retry. This works much better!

Also did some other refactoring of the inner workings, making things clearer, more concise and faster.

@vowelparrot for review

Copy link
Contributor

@dev2049 dev2049 left a comment

Choose a reason for hiding this comment

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

one nit but otherwise lgtm

langchain/tools/powerbi/tool.py Outdated Show resolved Hide resolved
@dev2049 dev2049 added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label May 22, 2023
@dev2049
Copy link
Contributor

dev2049 commented May 22, 2023

@eavanvalkenburg ready to merge?

@eavanvalkenburg
Copy link
Contributor Author

never mind, good to go, thanks @dev2049

@dev2049 dev2049 merged commit 1cb04f2 into langchain-ai:master May 22, 2023
12 checks passed
@eavanvalkenburg eavanvalkenburg deleted the pbi_refinement branch May 23, 2023 17:50
@danielchalef danielchalef mentioned this pull request Jun 5, 2023
This was referenced Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm PR looks good. Use to confirm that a PR is ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants