Type hint errors

[wthueb]

Dropped v4.0.1 into my project, and decided to see what Pylance says doesn't work.

RadarrAPI.add_movie should take a db_id: Union[str, int] since IMDB IDs are strs and TMDB IDs are ints (or at least those are the types returned by RadarrAPI.lookup_movie). This propagates to RadarrAPI._movie_json and RadarrAPI.lookup_movie_by_tmdb_id.

RadarrAPI.get_tag_detail should return Union[list[dict[str, Any]], dict[str, Any]] since as the docstring points out, "Returns all tags or specific tag by database id with detailed information".

SonarrAPI.get_series should return Union[list[dict[str, Any]], dict[str, Any]]: "Returns all series in your collection or the series with the matching series ID if one is found."

Also, to import RadarrCommands, should that be exposed at the top level pyarr package? Having to do from pyarr.models.radarr import RadarrCommands seems a bit esoteric (as well as SonarrCommands).

[marksie1988]

Thanks, will take a look at those tomorrow morning 👍

[github-actions]

Branch 111-type_hint_errors created!

[marksie1988]

After refactoring, I remember why I did this with these pieces, its due to the way MyPy reads a Union, it will error if the response is anything other than the union, even though we would expect it to accept either an int or str.

it makes sense as you are saying it could be either a str or an int, but im currently looking at a way around this, possibly with assert isinstance()

pyarr/radarr.py:56: error: Argument 1 to "lookup_movie_by_tmdb_id" of "RadarrAPI" has incompatible type "Union[str, int]"; expected "int"
pyarr/radarr.py:58: error: Argument 1 to "lookup_movie_by_imdb_id" of "RadarrAPI" has incompatible type "Union[str, int]"; expected "str"

seems others had this issue:

• Type error when assigning Union to one of the Union's types python/mypy#7183

[wthueb]

Yes I'm pretty sure the only way is to do an isinstance check. It feels a bit wrong, but it's the standard. Most APIs wouldn't make the distinction between returning an array or a single instance, as if there's just a single object it would return [obj] however we have to deal with what we're given with the APIs.

[wthueb]

Mostly unrelated to this, I think there's a mixup between using db_id as a parameter like in RadarrAPI.add_movie (TMDB/IMDB ID based on tmbd: bool) and using id_ as a parameter in RadarrAPI.get_movie (also a TMDB ID, no support for IMDB ID). Meanwhile, RadarrAPI.del_movie takes an id_ which is a Radarr database ID (movie_json['id']) instead of a TMDB/IMDB ID. Not sure if this applies elsewhere in the codebase/with other APIs, these are just the functions I've noticed the mismatch.

[wthueb]

Ah, one last thing. RadarrAPI.get_movie_by_movie_id is annotated as list[dict[str, Any]] when it should just be dict[str, Any].

[github-actions]

Branch 111-type_hint_errors created!

[marksie1988]

Mostly unrelated to this, I think there's a mixup between using db_id as a parameter like in RadarrAPI.add_movie (TMDB/IMDB ID based on tmbd: bool) and using id_ as a parameter in RadarrAPI.get_movie (also a TMDB ID, no support for IMDB ID). Meanwhile, RadarrAPI.del_movie takes an id_ which is a Radarr database ID (movie_json['id']) instead of a TMDB/IMDB ID. Not sure if this applies elsewhere in the codebase/with other APIs, these are just the functions I've noticed the mismatch.

Yes I agree there are some consistency issues with the naming of args etc. I will go through this at some point. However based on this I think the next think I look into will be setting up tests, not something ive done before but think it is necessary to avoid a lot of these issues.