Skip to content

add missing type hints in bedrock embedder - #240

Merged
codeforstartups merged 4 commits into
codeforstartups:developmentfrom
ramashishmaurya:my-new-feature
Sep 25, 2026
Merged

codeforstartups merged 4 commits into
codeforstartups:developmentfrom
ramashishmaurya:my-new-feature

Conversation

@ramashishmaurya

Copy link
Copy Markdown
Contributor

Description

Added missing Any and dict[str, Any] type hints to boto_session and body in the Bedrock embedder to improve code quality and fix linter warnings.

Related issue

None

Changes

  • Added from typing import Any
  • Added : Any | None = None to boto_session parameters
  • Added dict[str, Any] to body inside _invoke

Testing

  • Tests pass locally
  • Ruff checks pass
  • Documentation updated, if applicable

Checklist

  • My changes are focused and relevant to this pull request.
  • I have added or updated tests where appropriate.
  • I have reviewed my changes for unrelated modifications.
  • I have updated documentation where necessary.

Comment thread src/dynavec/eval/chart.py

@shivamm-gupta shivamm-gupta left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

added comments

@codeforstartups

Copy link
Copy Markdown
Owner

Thanks @ramashishmaurya — the bedrock type hints are welcome. One thing to fix: this PR also touches .github/workflows/publish.yml, but the change is a no-op (the pypa/gh-action-pypi-publish@release/v1 line is byte-identical — looks like a whitespace/line-ending edit). That accidental workflow-file change means the PR now requires workflow OAuth scope to merge (which automation lacks). Please revert publish.yml (e.g. git checkout origin/development -- .github/workflows/publish.yml) so the PR is just the bedrock.py + eval/chart.py type hints — then it can merge normally. Thanks!

@codeforstartups
codeforstartups merged commit 3e0fd7a into codeforstartups:development Sep 25, 2026
3 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