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

Add new colab notebook for AdalFlow data class implementation #226

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

ajithvcoder
Copy link
Contributor

Added a new notebook for dataclass tutorial based on following issues

@ajithvcoder ajithvcoder changed the title feat: add new notebook for AdalFlow data class implementation Add new colab notebook for AdalFlow data class implementation Oct 9, 2024
@ajithvcoder
Copy link
Contributor Author

@liyin2015 let me know if the notebook is fine

@@ -0,0 +1,794 @@
{
Copy link
Collaborator

@Sylph-AI Sylph-AI Oct 19, 2024

Choose a reason for hiding this comment

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

Line #5.        box_office: Optional[float] = field(

No need to use Optional. As any field in dataclass that comes with default or default_factory is an optional field. Sometimes using Optional will confuse the LLM or unable to support complicated data structure


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have addressed the issue

@@ -0,0 +1,794 @@
{
Copy link
Collaborator

@Sylph-AI Sylph-AI Oct 19, 2024

Choose a reason for hiding this comment

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

Try to use <USER></USER> to replace User: This fits more on how LLMs are using special tokens


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have addressed the issue

@liyin2015
Copy link
Member

liyin2015 commented Oct 19, 2024

@ajithvcoder This is great work to show more complicated dataclass structure. I have left comments and basically need to add "No optional" in the dataclass. And please adjust to this new info and rerun the test again. And then it should be good to go!

Also, please rebase to the main as the current base failed on tests

Copy link
Member

@liyin2015 liyin2015 left a comment

Choose a reason for hiding this comment

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

check out comments

@ajithvcoder
Copy link
Contributor Author

ajithvcoder commented Oct 20, 2024

@liyin2015 @Sylph-AI i will do the required changes but it wont pass the ci/cd because there is no quotes here . i have addressed similar issues in this pr #227 .
root cause: 3455b03

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ajithvcoder
Copy link
Contributor Author

@liyin2015 i have made the changes kindly review, its a single file which is not in Adalflow/main branch so i didnt rebase it. Also it has successfully passed the checks

@ajithvcoder
Copy link
Contributor Author

found a method for rebasing and i have rebased it also

Copy link
Member

@liyin2015 liyin2015 left a comment

Choose a reason for hiding this comment

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

looking good.

@liyin2015 liyin2015 merged commit 978f9b1 into SylphAI-Inc:main Oct 28, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants