Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix type hints in artefact_store #6

Open
hiker opened this issue Jul 12, 2024 · 0 comments
Open

Fix type hints in artefact_store #6

hiker opened this issue Jul 12, 2024 · 0 comments

Comments

@hiker
Copy link
Owner

hiker commented Jul 12, 2024

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??


OK, the error is not detected because the function isn't typed (--> 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]

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

jasonjunweilyu added a commit that referenced this issue Jul 15, 2024
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

No branches or pull requests

1 participant