ioos / erddapy

Python interface for ERDDAP
https://ioos.github.io/erddapy/
BSD 3-Clause "New" or "Revised" License
77 stars 30 forks source link

fix type output of _griddap_get_constraints #186

Closed ocefpaf closed 3 years ago

ocefpaf commented 3 years ago

@callumrollo I fixed the output type info but there are still a few other places to fix:

erddapy/erddapy.py:460: error: Argument 1 to "_griddap_check_constraints" has incompatible type "Optional[Dict[Any, Any]]"; expected "Dict[Any, Any]"
erddapy/erddapy.py:461: error: Argument 1 to "_griddap_check_variables" has incompatible type "Union[List[str], Tuple[str], None]"; expected "List[Any]"
erddapy/erddapy.py:472: error: Item "None" of "Union[List[str], Tuple[str], None]" has no attribute "__iter__" (not iterable)
erddapy/erddapy.py:474: error: Item "None" of "Union[List[str], Tuple[str], None]" has no attribute "__iter__" (not iterable)
erddapy/erddapy.py:476: error: Value of type "Optional[Dict[Any, Any]]" is not indexable

I'll send a few extra commits here to fix those ASAP but feel free to tackle them if you want to.

review-notebook-app[bot] commented 3 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

ocefpaf commented 3 years ago

We are down to one!

erddapy/erddapy.py:474: error: Item "None" of "Union[List[str], Tuple[str], None]" has no attribute "__iter__" (not iterable)
Found 1 error in 1 file (checked 13 source files)
ocefpaf commented 3 years ago

@callumrollo I narrowed it down to this part:

        (
            self.constraints,
            self.dim_names,
            self.variables,
        ) = _griddap_get_constraints(metadata_url)

again, my bad for not paying closer attention to the CI errors.

There we are clobbering the self.constraints, self.dim_names, and self.variables. The type check fails b/c None is possible and they are optional in the class. So we need to get non "self.` version of those first and update the class attribute only when it makes sense.

Do you want to tackle that? If so I'll merge this PR and let you work on it.

callumrollo commented 3 years ago

I'll have a go! Still finding Type checking a little mystifying, but I'll learn from the examples you've made. Good to merge this one now