Skip to content

Enhancement: Add type hints to journal model datastore functions - #1096

Open
inchara13kuppal wants to merge 1 commit into
sugarlabs:masterfrom
inchara13kuppal:feature/model-type-hints
Open

inchara13kuppal wants to merge 1 commit into
sugarlabs:masterfrom
inchara13kuppal:feature/model-type-hints

Conversation

@inchara13kuppal

Copy link
Copy Markdown

Fixes #1095

Adds PEP 484 type hints to the core write function in src/jarabe/journal/model.py. This explicitly defines the metadata parameter as a dict (along with boolean/string flags), laying the groundwork for strict type safety and data validation required for upcoming backend API middleware integrations.

@chimosky

Copy link
Copy Markdown
Member

In the future, please avoid opening an issue and a PR, just open a PR. It's noise for the maintainers.

That being said, it'll be great if this isn't done for just one part of our core APIs.

This is quite the effort and if you want to embark on it then that's okay.

@inchara13kuppal

Copy link
Copy Markdown
Author

Thanks for the guidance on the workflow, @chimosky ! Understood on the Issue and PR noise - I will stick to just PRs for these kinds of enhancements going forward.

I started with jarabe/journal/model.py specifically because I am currently finalizing my GSoC 2026 proposal for the AI Reflection in the Sugar Journal project. I needed to ensure the metadata dictionary was strictly typed so I could cleanly intercept it for the FastAPI middleware validation.

That being said, I would gladly take on the effort of expanding these type hints across the rest of the core APIs to ensure consistency.

Should I expand this current PR to cover the rest of the jarabe.journal module, or would you prefer I do this systematically across the codebase in separate, smaller PRs?

@chimosky

Copy link
Copy Markdown
Member

Thanks for the guidance on the workflow, @chimosky ! Understood on the Issue and PR noise - I will stick to just PRs for these kinds of enhancements going forward.

We usually open issues for things we don't intend to fix, if you intend to fix something then fix it.

Should I expand this current PR to cover the rest of the jarabe.journal module, or would you prefer I do this systematically across the codebase in separate, smaller PRs?

One PR per repo is best for this, makes it easier to test and review.

@inchara13kuppal

Copy link
Copy Markdown
Author

Thanks for the guidance on the workflow, @chimosky ! Understood on the Issue and PR noise - I will stick to just PRs for these kinds of enhancements going forward.

We usually open issues for things we don't intend to fix, if you intend to fix something then fix it.

Should I expand this current PR to cover the rest of the jarabe.journal module, or would you prefer I do this systematically across the codebase in separate, smaller PRs?

One PR per repo is best for this, makes it easier to test and review.

Understood! I'll expand this current PR to cover the type hints for the rest of the core APIs across the repository. I will let you know once the comprehensive updates are pushed and ready for review.

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.

Enhancement: Add PEP 484 type hints to core journal datastore functions (model.py)

2 participants