Closed Giom-V closed 1 week ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-08-28T06:28:52Z ----------------------------------------------------------------
Can you change the references to "Gemini" to say "Gemini API"? Gemini is the consumer-facing chatbot, it's confusing, so we need to be clear (it's something marketing legal request we do).
(as a side note, I wonder if we could use structured JSON output schemas to make the output parsing simpler and stable. just a thought, not something for this change)
Giom-V commented on 2024-09-11T12:46:11Z ----------------------------------------------------------------
Done for the references to Gemini. I will think of structured output, it's a good idea.
Giom-V commented on 2024-09-11T13:53:04Z ----------------------------------------------------------------
For the structured output, since this PR is already complicated, I propose to do it in a second one if that's ok with you.
markmcd commented on 2024-09-12T08:09:47Z ----------------------------------------------------------------
do it in a second one if that's ok with you.
yeah for sure - sorry, I was just thinking out loud!
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-08-28T06:28:52Z ----------------------------------------------------------------
Typo: examples
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-08-28T06:28:53Z ----------------------------------------------------------------
Line #2. # Adapted from https://colab.research.google.com/drive/1bjGbrtjE_Ugc3YpIvQMtkqsiMPqIp89W#scrollTo=02GJS3Zv4JAu written by Andreas Steiner and Gabriel Antoine le Roux
If this work is derived from that link, you'll need to include their copyright line in the header at the top of this file along with ours.
Do you have an example for that? I haven't changed this part of the code but I agree we should do it right...
_markmcd commented on 2024-09-12T08:21:00Z_ ----------------------------------------------------------------It may be sufficient to just include the copyright line from that Drive URL at the top of our file but I'll send an email to people who can answer with authority. I'll CC you.
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-08-28T06:28:54Z ----------------------------------------------------------------
Nit: Let's
is first-person. Now ask the model...
works fine, and Also ask for ...
Giom-V commented on 2024-09-11T12:52:10Z ----------------------------------------------------------------
Fixed
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-08-28T06:28:55Z ----------------------------------------------------------------
Line #1. url = "https://github.com/geminidemospatial/SpatialDemo/blob/main/gem_dogs.png?raw=true"
What are these images? There's no license in the repo, so the owner retains copyright, and we are not permitted to distribute them (e.g. by embedding them in a notebook and uploading to github)
If they are a Googler you could reach out and ask them to document the repo properly so we can use it?
I have absolutely no idea who it is, so I guess I'll have to change the pictures.
_Giom-V commented on 2024-09-11T13:53:25Z_ ----------------------------------------------------------------Replaced with a new picture from wikipedia.
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-08-28T06:28:55Z ----------------------------------------------------------------
Nits:
our
(first person), and I
newest
, refer to the specific model (so the content is timeless)
Giom-V commented on 2024-09-11T12:55:22Z ----------------------------------------------------------------
Fixed
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-08-28T06:28:56Z ----------------------------------------------------------------
If we're distributing these images (e.g. by including them in a notebook we upload to GitHub), we need to comply with their license (which appears to be MIT?).
Giom-V commented on 2024-09-11T12:56:46Z ----------------------------------------------------------------
Not sure what it entails to be honest. I'll talk to you directly about that.
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-08-28T06:28:57Z ----------------------------------------------------------------
Nit: Gemini API discovery
Giom-V commented on 2024-09-11T12:56:53Z ----------------------------------------------------------------
Fixed
Left some review feedback - mostly little nits but there are a couple of licensing questions we need to answer too.
Done for the references to Gemini. I will think of structured output, it's a good idea.
View entire conversation on ReviewNB
Do you have an example for that? I haven't changed this part of the code but I agree we should do it right...
View entire conversation on ReviewNB
I have absolutely no idea who it is, so I guess I'll have to change the pictures.
View entire conversation on ReviewNB
Not sure what it entails to be honest. I'll talk to you directly about that.
View entire conversation on ReviewNB
For the structured output, since this PR is already complicated, I propose to do it in a second one if that's ok with you.
View entire conversation on ReviewNB
do it in a second one if that's ok with you.
yeah for sure - sorry, I was just thinking out loud!
--- View entire conversation on ReviewNBIt may be sufficient to just include the copyright line from that Drive URL at the top of our file but I'll send an email to people who can answer with authority. I'll CC you.
View entire conversation on ReviewNB
View / edit / reply to this conversation on ReviewNB
markmcd commented on 2024-09-12T08:29:30Z ----------------------------------------------------------------
This cell has a weird error output, and the next one has an error too. Make sure they get cleaned up before merging.
Giom-V commented on 2024-09-13T09:42:34Z ----------------------------------------------------------------
I updated the outputs. It should be better now.