Skip to content

Updated Providing_base_cases.ipynb to use google-genai #643

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Shi-pra-19
Copy link

Updated module google-generativeai to use google-genai
Refactored imports

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added status:awaiting review PR awaiting review from a maintainer component:examples Issues/PR referencing examples folder labels Apr 3, 2025
@Giom-V Giom-V self-assigned this Apr 3, 2025
@@ -6,7 +6,7 @@
"id": "gy3KpnNx_Jl4"
Copy link
Collaborator

@Giom-V Giom-V Apr 3, 2025

Choose a reason for hiding this comment

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

Line #2.    You are an assistant that helps tourists around the world to plan their vacation. Your responsibilities are:

Can you add some indentation to improve readability? Like

instructions = """
     You are an assistant that helps tourists around the world to plan their vacation. Your responsibilities are:
     1. Helping book the hotel.
     2. Recommending restaurants.
     3. Warning about potential dangers.

     If other request is asked return "I cannot help with this request."
"""

Reply via ReviewNB

@@ -6,7 +6,7 @@
"id": "gy3KpnNx_Jl4"
Copy link
Collaborator

@Giom-V Giom-V Apr 3, 2025

Choose a reason for hiding this comment

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

Line #1.    MODEL_ID="gemini-2.0-flash" # @param ["gemini-2.0-flash-lite","gemini-2.0-flash","gemini-2.5-pro-exp-03-25"] {"allow-input":true, isTemplate: true}

We don't need to define the model all the time. I think once at the beginning of the notebook is enough.


Reply via ReviewNB

@@ -6,7 +6,7 @@
"id": "gy3KpnNx_Jl4"
Copy link
Collaborator

@Giom-V Giom-V Apr 3, 2025

Choose a reason for hiding this comment

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

Line #1.    print("ON TOPIC:", on_topic_response.text)

Can you use display(Markdown()) to improve the rendering of the markdown?


Reply via ReviewNB

@@ -6,7 +6,7 @@
"id": "gy3KpnNx_Jl4"
Copy link
Collaborator

@Giom-V Giom-V Apr 3, 2025

Choose a reason for hiding this comment

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

Can you add indentation here as well?


Reply via ReviewNB

@@ -6,7 +6,7 @@
"id": "gy3KpnNx_Jl4"
Copy link
Collaborator

@Giom-V Giom-V Apr 3, 2025

Choose a reason for hiding this comment

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

Line #1.    print('''## Specified genre:

Can you use display(Markdown()) to improve the rendering of the markdown? I also don't think the line breaks are needed.


Reply via ReviewNB

@Giom-V
Copy link
Collaborator

Giom-V commented Apr 3, 2025

Thanks for the submission @Shi-pra-19! It's the first one if I'm not mistaken 🥳

I just added a couple of minor comments to make the notebook easier to understand.

Can you also check the format failures. It is likely because you haven't run the formatting script.

Thanks again!

@Shi-pra-19
Copy link
Author

@Giom-V Thank you for the review! This one is my second submission—my first one was #642. 🥳

I’ve added indentation and run the formatting script. Let me know if there are any more improvements needed.

Also, please take a look at #642 as well—I fixed the indentation there too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:examples Issues/PR referencing examples folder status:awaiting review PR awaiting review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants