Install typing package only for Python < 3.5

[cghawthorne]

Prevents conflicts with built-in typing library. Follows advice in python/typing#573

Modifies change introduced in #9915. @chadrik @robertwb

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Choose reviewer(s) and mention them in a comment (R: @username).
• Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
• Update CHANGES.md with noteworthy changes.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	SDK	Apex	Dataflow	Flink	Gearpump	Samza	Spark
Go		---	---		---	---
Java
Python		---			---	---
XLang	---	---	---		---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website
Non-portable
Portable	---		---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

[chadrik]

Seems reasonable to me.

[cghawthorne]

Just for completeness, here's an example where the current setup was causing a problem: https://travis-ci.org/tensorflow/magenta/builds/647132754

[robertwb]

Has the builtin typing package evolved since 3.5 in ways that the external library has backported (and that we use)?

[cghawthorne]

My understanding from the bug linked above is that the typing external library is intended only to be used with versions of python (<3.5) that don't already have it built in and that it causes problems when it overrides built-in versions (python >3.5).

[chadrik]

I had a chance to look at the linked issues a bit more, and I take back my original assessment for the moment.

Has the builtin typing package evolved since 3.5 in ways that the external library has backported (and that we use)?

It has changed a lot, and I'd say it's highly likely that we're relying on features from newer typing libraries. And if we were to change to using the stdlib for typing it would be incredibly easy for a developer to use a feature in a later version of typing without knowing it (which may or may not be caught by tests). You would basically have to keep a copy of the 3.5 version of typing around and constantly refer back to it.

Here's a diff (search for src/typing.py):

python/typing@3.5.3.0...3.7.4.1

Just for completeness, here's an example where the current setup was causing a problem: https://travis-ci.org/tensorflow/magenta/builds/647132754

What I'm confused about is that you're testing in python 3.7.1, and beam's minimum version of typing is 3.7.0 and the max version on pypi is 3.7.4.1. So in the range 3.7.0 .. 3.7.4.1. I checked the diffs between that range and almost nothing changed about the typing library in that release range (added SupportsIndex), which seems to indicate that the version of typing pulled in by beam should be identical (or nearly) to the stdlib for 3.7.1. Is there a chance you're picking up an older version of typing from another library? IIRC, pip does a shit job with transitive dependencies.

[cghawthorne]

The Travis log includes the full installation history, so I'm pretty sure we're picking up typing-3.7.4.1 and typing-extensions-3.7.4.1.

The thing that triggered the problem for us was the beam 2.19.0 release. Maybe that started referring to a new set of types that caused problems? For now, we're just avoiding that release, which stopped the error from occurring (magenta/magenta@6ff823d).

[chadrik]

Do you have a log indicating which version of typing gets picked up with 2.18.0? I assume it's the same version. I think you're right that

Maybe that started referring to a new set of types that caused problems?

The exception is not coming from Beam, so it's unclear how using a feature of the typing module in Beam could be causing this issue, especially considering the typing module doesn't do much.

I'm not saying the problem isn't caused by Beam, but the evidence has left me pretty perplexed.

[aaltay]

drive by comment, this also seems related to the issue in (conda-forge/apache-beam-feedstock#21).

Do we know, if we are actually depending on the newer features from the typing libraries?

[chadrik]

Do we know, if we are actually depending on the newer features from the typing libraries?

The typing features added since 3.5.3 are:

• ContextManager
• SupportsBytes
• SupportsComplex
• SupportsIndex
• Counter
• Deque
• NoReturn

I'm pretty sure I did not use any of these.

There were significant runtime changes to the typing module, but the impact of these is impossible to predict from looking at the diff, and some of them may be beneficial while others are detrimental (hence this PR).

@cghawthorne can you remove the change to typing_extensions, its absence is causing test failures. This module is required for Protocol, which did not get added to typing until later versions (3.7.something).

[chadrik]

Back with some more info.

I'm increasingly of the opinion that this PR is the right way to go, but merging it should be contingent upon dropping support for python versions less than 3.5.3.

Here's why

1. pip install typing does not necessarily override the stdlib typing, because site-packages comes after the stdlib directories on sys.path.

2. There were substantial changes made to typing from 3.5.0 to 3.5.3. Here's what was added (the list in my previous comment was for after 3.5.3):

   • ClassVar
   • Type
   • DefaultDict
   • FrozenSet
   • NewType
   • Text
   • TYPE_CHECKING

   We're using most of these, so the code as it stands right now will fail for python 3.5.0 or 3.5.1. I think it's untenable to remove the use of these features, so we must choose a minimum point release to support.

3. Our tests are running against python 3.5.2, and I just ran into a bug with the typing module in 3.5.2 that was fixed in 3.5.3:

   https://scans.gradle.com/s/m7i4h44oi4cru/console-log?task=:sdks:python:test-suites:tox:py35:testPy35Cython

4. python 3.5.3 has the most up-to-date version of typing until python 3.6.1

Therefore, I think we'll be in pretty good shape if we update our test runners to use python 3.5.3 and then merge this PR.

[aaltay]

Thank you @chadrik. This is great information. I agree with your conclusion.

/cc @tvalentyn

[tvalentyn]

Thanks. The main problem with setting 3.5.3 as minimal required version is that Beam Jenkins workers are stuck with 3.5.2 right now, but we have an AI to address that: https://issues.apache.org/jira/browse/BEAM-8152. cc: @mwalenia @kamilwu @yifanzou .

We can skip tests failing on 3.5.2 if Python 3.5 version is less than 3.5.3 in the meantime. Do we have a lot of tests that are affected?

[chadrik]

We can skip tests failing on 3.5.2 if Python 3.5 version is less than 3.5.3 in the meantime. Do we have a lot of tests that are affected?

Not yet. I was about to introduce one on another PR, but if the switch to 3.5.3 is going to be awhile I can work around it. Is there Jira issue that I can reference in the code?

[tvalentyn]

@chadrik you can reference https://issues.apache.org/jira/browse/BEAM-9372.

[cghawthorne]

Anything I can help with here? I'm not completely following what all the blocking items are at this point.

[tvalentyn]

retest this please

[tvalentyn]

Rerunning the tests to see which tests failed on this PR. Requiring 3.5.3 is blocked on BEAM-8152. Would

    'typing>=3.7.0,<3.8.0; python_version < "3.5.3"',
    'typing-extensions>=3.7.0,<3.8.0; python_version < "3.5.3"',

be a possible workaround?

[tvalentyn]

So the tests fail because typing_extensions.Protocol is used in fn_api_runner.py, so we would need to make change there; from the discussion above @chadrik was thinking this through. @chadrik - do you have any suggestions how to reconcile dropping typing backports and using typing_extensions.Protocol ? Is there a concurrent change you are working on?

[chadrik]

typing_extensions is always required. I think the requirements should look like this:

    'typing>=3.7.0,<3.8.0; python_version < "3.5.3"',
    'typing-extensions>=3.7.0,<3.8.0

[chadrik]

typing_extensions is always required.

I guess I should say, it's always required until such a time as Protocol and Literal are both included in typing and we drop versions of python3 that don't include a new enough version of typing.

[tvalentyn]

typing_extensions is always required. I think the requirements should look like this:

    'typing>=3.7.0,<3.8.0; python_version < "3.5.3"',
    'typing-extensions>=3.7.0,<3.8.0

SGTM; @cghawthorne - could you please give it a try? Let's see if tests are passing.

[cghawthorne]

Updated!

[tvalentyn]

retest this please

[tvalentyn]

Sigh, Jenkins is not paying heed again.

[tvalentyn]

retest this please

[tvalentyn]

@cghawthorne Thanks for the changes, we can merge once we have a green test signal.

[tvalentyn]

Pinged https://issues.apache.org/jira/browse/INFRA-19836 and reached out to asfinfra via Slack regarding next steps there.

[tvalentyn]

Run Python PreCommit

[tvalentyn]

Run Python PreCommit

[tvalentyn]

ok, triggered now..

[tvalentyn]

Run Python 3.6 PostCommit

[tvalentyn]

Run Python 3.7 PostCommit

[tvalentyn]

Run Python 3.5 PostCommit

[kamilwu]

Run Python PreCommit

[kamilwu]

Run Python 3.7 PostCommit

[tvalentyn]

I checked that my commands actually triggered postcommits and they passed:

• https://builds.apache.org/job/beam_PostCommit_Python37_PR/98/
• https://builds.apache.org/job/beam_PostCommit_Python36_PR/56/
• https://builds.apache.org/job/beam_PostCommit_Python35_PR/103/

The most recent failure on the PR is a flake: https://issues.apache.org/jira/browse/BEAM-9484