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

ARROW-8175: [Python] Setup type checking with mypy #6676

Closed
wants to merge 3 commits into from

Conversation

xhochy
Copy link
Member

@xhochy xhochy commented Mar 20, 2020

No description provided.

@github-actions
Copy link

@xhochy xhochy force-pushed the ARROW-8175 branch 3 times, most recently from 70aae94 to 89292ca Compare March 23, 2020 12:59
@xhochy
Copy link
Member Author

xhochy commented Mar 23, 2020

The CI failure (C#) should be unrelated.

@alippai
Copy link
Contributor

alippai commented May 18, 2020

Can I help landing mypy support with anything? (I noticed that generated docs/interfaces are not in a good shape, with VSCode/PyCharm they are misleading, I assume this would help)

@xhochy
Copy link
Member Author

xhochy commented May 19, 2020

Things that would help:

  • One of @wesm, @pitrou or @jorisvandenbossche should state their opinion on this ;)
  • We would need to have better support in Cython for type annotations. As the main share of our code base is in Cython and stub generation for Cython files is not really in a good shape, the majority of our API won't be typed at all.

@alippai
Copy link
Contributor

alippai commented May 19, 2020

So ideally a follow-up PR would generate mypy check-able code from this?

Instead of the current one:
image
(this is the CTRL+Q preview from PyCharm)

@xhochy
Copy link
Member Author

xhochy commented May 19, 2020

So ideally a follow-up PR would generate mypy check-able code from this?

Instead of the current one:
image
(this is the CTRL+Q preview from PyCharm)

Yes but needs to be fixed on the Cython or mypy-stubs side.

@jorisvandenbossche
Copy link
Member

What's the status of mypy support of cython? (not very familiar with this)

@alippai
Copy link
Contributor

alippai commented May 19, 2020

What's the status of mypy support of cython? (not very familiar with this)

Are you looking for something like this? python/mypy#8631

@jorisvandenbossche
Copy link
Member

Maybe I was more looking for a human summary and its implications for this PR ;)

But since that is an open PR: there is right now no mypy support whatsoever for cython? So that would mean that, for now, we simply can't type the part of pyarrow that is coded in cython?
(maybe I just misinterpreted @xhochy's "Things that would help", as the listed "better support in Cython for type annotations" is not something that is actionable in pyarrow at the moment?)

@emkornfield
Copy link
Contributor

@jorisvandenbossche @xhochy next steps here?

@jorisvandenbossche
Copy link
Member

If we are OK with this being rather limited for now (limited to the part of pyarrow that is written in python, as I understand cython is not yet supported), then I am fine with moving forward with this.

@emkornfield
Copy link
Contributor

I think any automation we can get is useful and we can file a follow-up JIRA to check periodically for cython. Unless I'm missing something even python only support is a strict improvement?

@jorisvandenbossche
Copy link
Member

Yes, I think it will be a strict improvement, but it might also not be doing much validation yet, since a lot of the pyarrow objects / functions will just be typed with "Any".
Now, I am not experienced enough with mypy to make the tradeoff in this case, but so if someone wants to move this forward and update the PR, that's fine for me.

@emkornfield
Copy link
Contributor

@xhochy in the interest of triaging issues, I'm going to close this for now, could you rebase/reopen when it is ready?

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

Successfully merging this pull request may close these issues.

4 participants