Skip to content

Conversation

@Samoed
Copy link

@Samoed Samoed commented Aug 22, 2025

No description provided.

@Samoed
Copy link
Author

Samoed commented Aug 28, 2025

Hi @seanmacavaney can you review?

@Samoed
Copy link
Author

Samoed commented Sep 5, 2025

@cmacdonald @seanmacavaney can you review?

@cmacdonald
Copy link
Collaborator

Thanks for this, nice work.

We should add a mypy GHA that verifies the types. Can you take a style.yml file from another of our projects?

@Samoed
Copy link
Author

Samoed commented Sep 5, 2025

Added ruff config

@Samoed
Copy link
Author

Samoed commented Sep 5, 2025

I tried to add mypy, but it raises some errors. I can try to fix it

@cmacdonald
Copy link
Collaborator

Here is the mypy output:

py/__init__.py:9: error: Cannot find implementation or library stub for module named "numpy"  [import-not-found]
py/__init__.py:9: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
py/__init__.py:10: error: Cannot find implementation or library stub for module named "pytrec_eval_ext"  [import-not-found]
py/__init__.py:41: error: Need type annotation for "run"  [var-annotated]
py/__init__.py:70: error: Need type annotation for "qrel"  [var-annotated]
py/__init__.py:180: error: Need type annotation for "param_meas"  [var-annotated]
py/__init__.py:187: error: Item "None" of "Optional[Match[str]]" has no attribute "group"  [union-attr]

@cmacdonald
Copy link
Collaborator

added some comments - nearly there @Samoed !

@Samoed
Copy link
Author

Samoed commented Sep 5, 2025

Changed ruff to singe quotes

@cmacdonald
Copy link
Collaborator

LGTM. are you happy @seanmacavaney ?

@seanmacavaney
Copy link
Collaborator

To avoid increasing the min supported python version, I switched to Dict, List, Set from typing.

I think this is good to go, I'll merge and release.

Thanks a bunch @Samoed!

@seanmacavaney seanmacavaney merged commit b727b73 into terrierteam:master Sep 5, 2025
9 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