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 errors reported by mypy #64

Closed
ghost opened this issue Nov 11, 2019 · 6 comments
Closed

fix errors reported by mypy #64

ghost opened this issue Nov 11, 2019 · 6 comments
Assignees
Labels
consideration Future decision to be discussed

Comments

@ghost
Copy link

ghost commented Nov 11, 2019

Found 75 errors in 23 files (checked 43 source files)

@felix-hilden
Copy link
Owner

felix-hilden commented Nov 11, 2019

I have no experience with mypy. So I'm not sure of the impact of trying to conform is, but I'll look into it.

@felix-hilden felix-hilden added bug Something isn't working consideration Future decision to be discussed labels Nov 11, 2019
@felix-hilden felix-hilden self-assigned this Nov 15, 2019
@felix-hilden
Copy link
Owner

Yeah, I tested it out, but at a quick glance none of the errors were valid. Seems like a bad move. For example, all models that had a None default were errors because a none type is of course incompatible with whatever the field is supposed to be.

@felix-hilden felix-hilden added the invalid This doesn't seem right label Nov 15, 2019
@ghost
Copy link
Author

ghost commented Nov 15, 2019

for example:
spotipy/model/user.py:12: error: Incompatible types in assignment (expression has type "None", variable has type "Followers")

I think typing.Optional should be used:
followers: Optional[Followers] = None

(and also document on what situation followers is None?)

@felix-hilden
Copy link
Owner

I created an issue of the model types. They need some work. But here are some more points of concern, just a few of the first ones, and I can't see the reason to fix them. That's just a whole lot of unnecessary type hints.

  • client/browse/__init__:226 : Incompatible types in assignment (str -> int) where a simple dict is populated. seed_artists surely is and is supposed to be a string. This goes for the other assignments as well.
  • client/artist:73 : Incompatible types in assignment when parsing include_groups if they are not none. Having another variable would require an else: var = None statement and even then mypy would issue an error when assigning none to a string.
  • client/player : missing return statements. They are not needed, because None should be returned. Incompatible dict types in 166, which again are not valid complaints.

@ghost
Copy link
Author

ghost commented Nov 15, 2019

@felix-hilden
Copy link
Owner

On every such line? I'm sorry, it's a no. I'm all for using a static checker to find potential flaws but the code itself shouldn't bend to a linter's will. Maybe that's just me, but what a mess.

@felix-hilden felix-hilden removed invalid This doesn't seem right bug Something isn't working labels Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consideration Future decision to be discussed
Projects
None yet
Development

No branches or pull requests

1 participant