Skip to content

chore(mypy): add mypy type hints to distributions and testing modules#5404

Open
Elbehery wants to merge 1 commit intoogx-ai:mainfrom
Elbehery:20260401_add_type_hints_providers_dist
Open

chore(mypy): add mypy type hints to distributions and testing modules#5404
Elbehery wants to merge 1 commit intoogx-ai:mainfrom
Elbehery:20260401_add_type_hints_providers_dist

Conversation

@Elbehery
Copy link
Copy Markdown
Contributor

@Elbehery Elbehery commented Apr 1, 2026

What does this PR do?

Add type hints to make the following files pass strict mypy checking:

  • src/llama_stack/distributions/template.py: Add explicit type annotations to filtered_dict and filtered_list variables, rename config_class to config_cls to avoid shadowing, rename template variable to jinja_template to avoid shadowing, cast jinja2 render return value to str, add cast to typing imports
  • src/llama_stack/testing/api_recorder.py: Add string annotation for ResponseStorage forward reference, add type annotation for headers dict, add assertion for non-None storage, add type: ignore[method-assign] comments for all method monkey-patching operations

cc @leseb

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Apr 1, 2026
@Elbehery Elbehery force-pushed the 20260401_add_type_hints_providers_dist branch 4 times, most recently from 28f3d5a to 3e1ddf4 Compare April 1, 2026 14:31
@Elbehery Elbehery changed the title chore(mypy): remove distributions and other files from mypy exclude list chore(mypy): add mypy type hints to distributions and testing modules Apr 1, 2026
@Elbehery Elbehery force-pushed the 20260401_add_type_hints_providers_dist branch 3 times, most recently from 3421c11 to b11eabe Compare April 1, 2026 21:00
Copy link
Copy Markdown
Collaborator

@leseb leseb left a comment

Choose a reason for hiding this comment

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

let's not rename variables if not necessary

@Elbehery Elbehery force-pushed the 20260401_add_type_hints_providers_dist branch from b11eabe to fc58a62 Compare April 2, 2026 15:08
@Elbehery
Copy link
Copy Markdown
Contributor Author

Elbehery commented Apr 2, 2026

let's not rename variables if not necessary

fixed 👍🏽

@Elbehery Elbehery force-pushed the 20260401_add_type_hints_providers_dist branch 8 times, most recently from 82c5303 to 6d1df48 Compare April 9, 2026 14:03
Copy link
Copy Markdown
Collaborator

@leseb leseb left a comment

Choose a reason for hiding this comment

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

this PR is mostly ignoring and not really fixing, at least let's leave an explanation on why we ignore

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Apr 13, 2026

This pull request has merge conflicts that must be resolved before it can be merged. @Elbehery please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Apr 13, 2026
@Elbehery Elbehery force-pushed the 20260401_add_type_hints_providers_dist branch from 6d1df48 to dc3816b Compare April 13, 2026 11:52
@mergify mergify Bot removed the needs-rebase label Apr 13, 2026

PyPDFFileProcessorAdapter.process_file = _original_methods["pypdf_process_file"]

# Restore aiohttp method
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.

removed comment?

run_config_env_vars=self.run_config_env_vars,
default_models=default_models,
run_configs=list(self.run_configs.keys()),
return cast(
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.

which err does this fix?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

to fix

Returning Any from function declared to return "str" [no-any-return] 

I got those

src/llama_stack/distributions/template.py:385: error: Returning Any from function declared to return "str"  [no-any-return]
src/llama_stack/testing/api_recorder.py:972: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
src/llama_stack/testing/api_recorder.py:1127: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
src/llama_stack/testing/api_recorder.py:1225: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
Found 1 error in 1 file (checked 401 source files)

then had to add the cast

@Elbehery Elbehery force-pushed the 20260401_add_type_hints_providers_dist branch 3 times, most recently from cd26c11 to 63909c4 Compare April 16, 2026 21:01
Add type hints to make the following files pass strict mypy checking:
- src/llama_stack/distributions/template.py: Add explicit type
  annotations to filtered dict and list variables, cast jinja2 render
  return value to str, add cast to typing imports
- src/llama_stack/testing/api_recorder.py: Add string annotation for
  ResponseStorage forward reference, add type annotation for headers
  dict, add assertion for non-None storage, add
  type: ignore[method-assign] comments for all method monkey-patching
  operations

Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>
@Elbehery Elbehery force-pushed the 20260401_add_type_hints_providers_dist branch from 63909c4 to 4ca60e4 Compare April 17, 2026 09:34
@Elbehery
Copy link
Copy Markdown
Contributor Author

cc @leseb

@Elbehery
Copy link
Copy Markdown
Contributor Author

@leseb any reviews here please ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants