hiker / fab

Flexible build system for scientific software
https://metomi.github.io/fab/
Other
1 stars 0 forks source link

Fix type hints in artefact_store #31

Closed hiker closed 1 month ago

hiker commented 2 months ago

As commented by @jasonjunweilyu :

test ArtefactStore.reset
test string as inputs for ArtefactStore.add, ArtefactStore.copy_artefacts, ArtefactStore.replace and CollectionGetter
test string as input for value of ArtefactStore.update_dict
test dict as input for add_files of ArtefactStore.replace
test str and ArtefactsGetter as inputs for CollectionGetter
potentially group tests together in classes, similar to class TestFilterBuildTrees

A quick change of using strs fails:

 git diff test_artefacts.py
diff --git a/tests/unit_tests/test_artefacts.py b/tests/unit_tests/test_artefacts.py
index a0f6bd4..d141ce7 100644
--- a/tests/unit_tests/test_artefacts.py
+++ b/tests/unit_tests/test_artefacts.py
@@ -29,7 +29,7 @@ def test_artefact_store_copy():
     '''Tests the add and copy operations.'''
     artefact_store = ArtefactStore()
     # We need paths for suffix filtering, so create some
-    a = Path("a.f90")
+    a = "a.f90"
     b = Path("b.F90")
     c = Path("c.f90")
     d = Path("d.F90.nocopy")
...
test_artefacts.py:54: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../source/fab/artefacts.py:108: in copy_artefacts
    self.add(dest, set(suffix_filter(self[source], suffixes)))
../../source/fab/util.py:237: in suffix_filter
    return list(filter(lambda fpath: fpath.suffix in suffixes, fpaths))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

fpath = 'a.f90'

>   return list(filter(lambda fpath: fpath.suffix in suffixes, fpaths))
E   AttributeError: 'str' object has no attribute 'suffix'

../../source/fab/util.py:237: AttributeError

And strange, if I try to prevent from strings being used (specifying Path instead of str as parameters do add in the ArtefactStore), the string is still accepted without any error. And confusingly: why is mypy quiet about providing a Path where the parameter is supposed to be a string??

hiker commented 2 months ago

OK, the error is not detected because the function isn't type (--> mypy ignores it). Adding --> None:

def test_artefact_store_copy() -> None:

then results in:

$ mypy source/ tests/unit_tests/test_artefacts.py 
tests/unit_tests/test_artefacts.py:38: error: Argument 2 to "add" of "ArtefactStore" has incompatible type "Path"; expected "str | list[str] | set[str]"  [arg-type]
tests/unit_tests/test_artefacts.py:42: error: List item 0 has incompatible type "Path"; expected "str"  [list-item]
tests/unit_tests/test_artefacts.py:42: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
tests/unit_tests/test_artefacts.py:43: error: List item 0 has incompatible type "Path"; expected "str"  [list-item]
tests/unit_tests/test_artefacts.py:43: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
hiker commented 2 months ago

Trying to consistently fix this in this test raises a lot of mypy warnings :(

hiker commented 1 month ago

Migrated to new repo.