Remove mypy ignore annotations #5
Conversation
fms-extras-requirements.txt
Outdated
| # for repeatable installs | ||
|
|
||
| torch==2.2.0 | ||
| transformers==4.35.0 |
There was a problem hiding this comment.
ibm-fms depends on these too right? I wonder if we should just get these indirectly as dependencies of ibm-fms?
There was a problem hiding this comment.
That's a good point.
We do have those dependencies in https://github.com/foundation-model-stack/fms-extras/blob/fdb1636de4261fd4102da659ab45d3fcc33fe8ef/setup.py#L13C1-L16C32 though (plus accelerate which I had missed).
This package might have its specific requirements on those libraries eventually, possibly stricter than FMS, but we could add them when needed instead of now.
There was a problem hiding this comment.
I updated this to only include accelerate as pinned dependency
There was a problem hiding this comment.
why fms-extras-requirements.txt instead of just the usual requirements.txt?
There was a problem hiding this comment.
A requirements.txt would be picked up by a casual pip install command, but I think that the requirements.txt which points to fms@main should only be for dev/test use cases.
Add two requirement files: - fms-extras-requirements.txt: it includes pinned dependencies except for ibm-fms that is taken from git, main branch - test-requirements.txt: it includes packages for testing and it points to fms-extras-requirements.txt too These are handy for test jobs, to install fms from main, so we don't depend on a release for testing purposes, at least for now. Adapt the mypy workflow to take advantage of the new requirement files. Signed-off-by: Andrea Frittoli <[email protected]>
The fms project now includes a (partial) py.typed file, which allows mypy testing to pass for the fms-extras package. Typing is incomplete on fms side in the hf package, so we continue to exclude the corresponding package on fms-extras to avoid failures. The extra testing uncoverved two issues in calico.py: - fms_extras/models/calico.py:205: error: "list[int]" has no attribute "values" [attr-defined] Fixed by removing the redundant values(), set(list[int]) works as it is. - fms_extras/models/calico.py:217: error: Incompatible types in assignment (expression has type Module, variable has type "CalicoBlock") [assignment] Fixed by introducing an extra variable instead of overriding the original variable with an incompatible type. Signed-off-by: Andrea Frittoli <[email protected]>
This PR is made of two commits:
Add requirement files for dev and test
Add two requirement files:
These are handy for test jobs, to install fms from main, so we don't depend on a release for testing purposes, at least for now.
Adapt the mypy workflow to take advantage of the new requirement files.
Remove mypy ignore annotations
The fms project now includes a (partial) py.typed file, which allows mypy testing to pass for the fms-extras package.
Typing is incomplete on fms side in the hf package, so we continue to exclude the corresponding package on fms-extras to avoid failures. The extra testing uncovered two issues in
calico.py:fms_extras/models/calico.py:205: error: "list[int]" has no attribute "values" [attr-defined]Fixed by removing the redundant values(), set(list[int]) works as it is.
fms_extras/models/calico.py:217: error: Incompatible types in assignment (expression has type Module, variable has type "CalicoBlock") [assignment]Fixed by introducing an extra variable instead of overriding the original variable with an incompatible type.
Signed-off-by: Andrea Frittoli [email protected]