[
https://issues.apache.org/jira/browse/BEAM-7746?focusedWorklogId=336474&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-336474
]
ASF GitHub Bot logged work on BEAM-7746:
----------------------------------------
Author: ASF GitHub Bot
Created on: 30/Oct/19 21:17
Start Date: 30/Oct/19 21:17
Worklog Time Spent: 10m
Work Description: chadrik commented on pull request #9056: [BEAM-7746]
Add python type hints
URL: https://github.com/apache/beam/pull/9056#discussion_r340825002
##########
File path: sdks/python/tox.ini
##########
@@ -160,16 +160,17 @@ commands =
[testenv:py27-lint]
# Checks for py2 syntax errors
deps =
+ -r build-requirements.txt
Review comment:
This is required if we want `tox` to work outside of gradle, and I really
really do. Using gradle to run tox tasks adds a lot of overhead and I kind of
hate it. gradle creates a complete virtualenv just to build an sdist, then it
creates another virtualenv to run tox (and IIRC it does this every time). If I
run tox directly it creates a single virtualenv for both sdist and the tox env
and it reuses that virtualenv for subsequent runs of tox. Gradle also just
seems slow to start up. Also, I can't pass individual tests to run when using
gradle. Let's just say that as a python developer, the gradle experience is
the thing I like least about the Beam project.
Yes, by adding the gen_proto requirements to `build-requirements.txt` and
installing that in the `setupVirtualenv` task I'm making that overhead worse,
but I certainly didn't start that problem: the code is currently installing
`tox` every time that the `sdist` task is run, which is pointless. In other
words, `setupVirtualenv` is currently installing the union of all possible task
requirements, and I'm extending that pattern.
I could add a feature to `setupVirtualenv` to provide a requirements file,
which would let us provide exactly the deps required for each task:
- `sdist`: grpcio-tools, mypy-protobuf, future
- `tox`: tox
That would end up a net win, even with the deps added in this PR, because
tox has a lot of deps. Should I do that here or in a followup PR?
I've been meaning to bring up some of these issues on the list. I assume
the reason for the two step test process (sdist + tox, each with its own
virtualenv) is to ensure that tox is using an sdist built in the same way every
time, using python2, just as it would be for distribution?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 336474)
Time Spent: 15h 20m (was: 15h 10m)
> Add type hints to python code
> -----------------------------
>
> Key: BEAM-7746
> URL: https://issues.apache.org/jira/browse/BEAM-7746
> Project: Beam
> Issue Type: New Feature
> Components: sdk-py-core
> Reporter: Chad Dombrova
> Assignee: Chad Dombrova
> Priority: Major
> Time Spent: 15h 20m
> Remaining Estimate: 0h
>
> As a developer of the beam source code, I would like the code to use pep484
> type hints so that I can clearly see what types are required, get completion
> in my IDE, and enforce code correctness via a static analyzer like mypy.
> This may be considered a precursor to BEAM-7060
> Work has been started here: [https://github.com/apache/beam/pull/9056]
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
