-
-
Notifications
You must be signed in to change notification settings - Fork 0
Calm Calatheas #22
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
base: main
Are you sure you want to change the base?
Calm Calatheas #22
Conversation
Co-authored-by: Copilot <[email protected]>
Add gpg to the devcontainer in order to support signed commits. See the guide on sharing [GPG keys with the devcontainer](https://code.visualstudio.com/remote/advancedcontainers/sharing-git-credentials#_sharing-gpg-keys) for details. For me the following scenario works: **On the host machine (WSL/Ubuntu):** - Install `gnupg2` - Generate a GPG key and register it in GitHub - Configure git as follows: ```shell git config --global commit.gpgsign true git config --global user.signingkey <your key id> ``` **In the devcontainer:** - Rebuild the container with the feature added in this PR (installing `gnupg2`)
Add [devenv.sh](https://devenv.sh/)/[direnv](https://direnv.net/) as an option for setting up a development environment. Comes with: - Python - uv - Node - pre-commit
Create a very basic skeleton app to be expanded on in the future, and some complimentary docs on how to run. Closes #9 --------- Co-authored-by: thijsfranck <[email protected]>
- The app now has a header, footer, and main body - Introduce bulma.css as a styling framework, as well as a custom font and color palette - Include a menu to switch themes for some interactivity
Add functionality to access the user's camera, visualize the feed on the screen and let the user capture an image. For now the image is saved to the users's device. This is placeholder behavior as the image should eventually feed into the object recognition. How to use: - Open the app and click the camera button at the bottom of the screen - The browser will request permission to use the camera, approve the request - The camera feed should now appear. - Make a pose and click the capture button 😁 - An snapshot taken from the camera stream should now be downloaded to your device Unfortunately I didn't figure out how to fake the camera for testing, so no automated tests are included.
I have added a test component (./app/calm_calatheas/components/description_test.py) as a proof of concept.
This PR links the camera functionality with the caption analysis, and sets up description generation.
This PR adds support for multiple pokemon to the app. They are stored in IndexedDB, which is an in-browser document database. Also included are some UI touch-ups.
Various bug fixes and cleaned up the UI. Also added code documentation ahead of the deadline.
Co-authored-by: thijsfranck <[email protected]>
Co-authored-by: papaya <[email protected]>
| class PokemonType(StrEnum): | ||
| """An enumeration of Pokemon types.""" | ||
|
|
||
| BUG = auto() | ||
| DARK = auto() | ||
| DRAGON = auto() | ||
| ELECTRIC = auto() | ||
| FAIRY = auto() | ||
| FIGHTING = auto() | ||
| FIRE = auto() | ||
| FLYING = auto() | ||
| GHOST = auto() | ||
| GRASS = auto() | ||
| GROUND = auto() | ||
| ICE = auto() | ||
| NORMAL = auto() | ||
| POISON = auto() | ||
| PSYCHIC = auto() | ||
| ROCK = auto() | ||
| STEEL = auto() | ||
| WATER = auto() |
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.
This appears elsewhere in the code--perhaps there was a way you could have defined this only once.
| @pytest.fixture(scope="session") | ||
| def compose() -> Generator[DockerCompose]: | ||
| """Return a Docker Compose instance.""" | ||
| with DockerCompose(context=Path(__file__).parent.absolute(), build=True) as compose: | ||
| yield compose |
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.
Excellent use of session-scoped fixtures.
| @pytest.mark.parametrize( | ||
| "path", | ||
| [ | ||
| Path(__file__).parent / "assets/elephant.jpg", |
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.
Did you intend to add more of these?
| [ | ||
| Path(__file__).parent / "assets/elephant.jpg", | ||
| ], |
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.
This appears several times. Perhaps you could have made one list of test jpgs to be shared between all these tests.
| def _prompt(messages: list[dict[str, str]]) -> tuple[str, str]: | ||
| """Prompt the model with the given messages and return the generated text.""" |
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.
The docstring should explain the significance of the two elements of the returned tuple (what do thinking_content and context represent?).
People who often use LLMs will probably recognize list[dict[str, str]]. If this code were intended to be read by general Python developers, I would have used a TypedDict:
import typing as t
class LlmMessage(t.TypedDict):
role: t.Literal['system', 'user', 'agent']
content: str|
Hey Calatheas (hope I'm pronouncing that right)! Congratulations on having made it to the top eleven of this year's code jam. I enjoyed getting to do a deep dive of your project so that I could provide some feedback for you. While I spent about three hours total reading your code, I have relatively few comments--this code is professional quality, and you evidently don't need me to help you. I chose to evaluate your project because I'm a language technology professional, and I was interested in how creatively you used a reasoning model for this task. (I read some of the thinking contexts to my coworkers, and they thought it was funny.) I noticed that in your code, you refer to the output of the image captioning model as the "user prompt". This is potentially confusing--a "user prompt" is usually text written by a user, not text that's derived from some other user-provided object. It would be more clear to have referred to this as "user caption", or something. I appreciate the documentation you wrote for standing up the project, and that you made it easy by using docker and uv. I would have liked to see more commits. 23 commits is quite few for the amount of code and functionality implemented here, though I know it can be hard to write atomic commits in nascent projects. Are you aware of I also observed that thisisfranck (aka TFBlunt) wrote quite a bit of the code. Speaking to you directly: I see from messages in the server that you were a proactive team leader, so I wonder how your leadership could have translated to a more even distribution of effort. Thank you for waiting so long for my review. I finished evaluating your code for judgement purposes before we decided the winners, but I didn't get around to writing my thoughts for you because it's been a tumultuous month here in Washington, DC. |
No description provided.