Skip to content

mypy: Fix erroring files. #127

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

Closed
roberthoenig opened this issue Sep 26, 2017 · 13 comments
Closed

mypy: Fix erroring files. #127

roberthoenig opened this issue Sep 26, 2017 · 13 comments

Comments

@roberthoenig
Copy link
Collaborator

Background

We have recently added mypy support to this repo, but many files are corrupted and throw errors on mypy.

Task

#126 updates the exclude_list in tools/run-mypy Each file in this list errors on mypy. We should fix each file and remove it from the list. zulip_bots/zulip_bots/bots is an exception, we don't enforce mypy annotations in this directory.

This task is great for beginners or/and those who want to get comfortable with mypy annotations.

@alenavolk
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

zulipbot commented Oct 6, 2017

Welcome to Zulip, @alenavolk! We just sent you an invite to collaborate on this repository at https://github.com/zulip/python-zulip-api/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

@alenavolk
Copy link
Collaborator

@derAnfaenger There's a bunch of files on that list that don't cause errors in my case (I tried removing them and running ./tools/run-mypy):

zulip/integrations/git/zulip_git_config.py
zulip/integrations/irc/irc_mirror_backend.py
zulip/integrations/perforce/zulip_perforce_config.py
zulip/integrations/perforce/git_p4.py
zulip/integrations/svn/zulip_svn_config.py
zulip/tests/__init__.py
zulip_botserver/tests/__init__.py
zulip_botserver/zulip_botserver/server.py

Could this be because mypy works differently with different Python versions? I use Python 3.6.

@roberthoenig
Copy link
Collaborator Author

Possible. We have a fixed version of mypy in the virtualenv that should work the same for everybody, are you using that one (0.521)?

@alenavolk
Copy link
Collaborator

@derAnfaenger Yep.

@alenavolk
Copy link
Collaborator

@derAnfaenger Do these files still cause errors if you run ./tools/run-mypy without excluding them? If so, could you specify what version of Python you have?

@roberthoenig
Copy link
Collaborator Author

Hrmm, this is strange: zulip_git_config.py contains no annotations at all and should therefore be caught by --disallow-untyped-defs, but it doesn't. Adding --disallow-untyped-calls as an argument catches the file, but also a bunch of other files not yet in the exclusion list.

I'd say that you should feel free to remove the files not caught by mypy, since there hasn't been any breaking change to run-mypy.

@alenavolk
Copy link
Collaborator

@derAnfaenger Hm, interesting. I'll focus on the rest of the files then. Thanks for the feedback!

@alenavolk
Copy link
Collaborator

Just a quick update. I managed to reproduce the errors from all excluded files by deleting the .mypy_cache dir from the project root.

@timabbott
Copy link
Member

@alenavolk ahh, that's unfortunate -- I would have thought mypy's caching would be smarter than that. Well, at least now you can start working on fixing the errors :)

@alenavolk
Copy link
Collaborator

Does it really make sense to annotate zulip/integrations/perforce/git_p4.py? It looks like a temporary solution that might get deleted (or replaced with a later version) in the future.

@timabbott
Copy link
Member

No, we should leave that in the exclude list forever, with a comment about how it's third-party code.

@zulipbot
Copy link
Member

Hello @alenavolk, you claimed this issue to work on it, but this issue and any referenced pull requests haven't been updated for 7 days. Are you still working on this issue?

If so, please update this issue by leaving a comment on this issue to let me know that you're still working on it. Otherwise, I'll automatically remove you from this issue in 3 days.

If you've decided to work on something else, simply comment @zulipbot abandon so that someone else can claim it and continue from where you left off.

Thank you for your valuable contributions to Zulip!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants