[ 
https://issues.apache.org/jira/browse/BEAM-8350?focusedWorklogId=327096&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-327096
 ]

ASF GitHub Bot logged work on BEAM-8350:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 11/Oct/19 22:10
            Start Date: 11/Oct/19 22:10
    Worklog Time Spent: 10m 
      Work Description: tvalentyn commented on issue #9725: [BEAM-8350] Upgrade 
to Pylint 2.4
URL: https://github.com/apache/beam/pull/9725#issuecomment-541241040
 
 
   From development perspective, it would be easiest to run linter using the 
highest version we support (for example, no need to exclude tests using new 
syntax)
   
   From compatibility perspective, it would be more reliable to run the linter 
using the lowest version we support. The argument that compatibility errors 
will be caught by tests is a reasonable argument, however I am not sure that 
all modules are covered by tests. Advantage of linter is that it parses all the 
codebase and will be able to spot incompatible syntax. We can consider running 
python2 linter ignoring all/most linter warnings, but still checking that 
linter is able to parse the files? Unfortunately, we would have to keep some 
files excluded if we do so. I think this is the price we have to pay to ensure 
we support older version of python. 
   
   Also, I think syntax compatibility problem will remain beyond Python 2, as 
new minor versions of Python 3 may introduce incompatible syntactic features. 
Do we want to make sure that Beam core modules do not use such features? I 
think the answer is yes. Can we guarantee that this will be enforced by tests? 
I think not until we enforce code coverage criteria in precommits. 
   
   Python 2/3 compatibility checks also add value, and in my opinion should be 
kept while Beam supports Python 2. For example, such checks currently catch  
division statements without  `from __future__ import division`. These can cause 
subtle errors that in the past caused performance regressions 
(https://issues.apache.org/jira/browse/BEAM-4858).  
 
----------------------------------------------------------------
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:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 327096)
    Time Spent: 4h  (was: 3h 50m)

> Upgrade to pylint 2.4
> ---------------------
>
>                 Key: BEAM-8350
>                 URL: https://issues.apache.org/jira/browse/BEAM-8350
>             Project: Beam
>          Issue Type: Improvement
>          Components: sdk-py-core
>            Reporter: Chad Dombrova
>            Assignee: Chad Dombrova
>            Priority: Major
>          Time Spent: 4h
>  Remaining Estimate: 0h
>
> pylint 2.4 provides a number of new features and fixes, but the most 
> important/pressing one for me is that 2.4 adds support for understanding 
> python type annotations, which fixes a bunch of spurious unused import errors 
> in the PR I'm working on for BEAM-7746.
> As of 2.0, pylint dropped support for running tests in python2, so to make 
> the upgrade we have to move our lint jobs to python3.  Doing so will put 
> pylint into "python3-mode" and there is not an option to run in 
> python2-compatible mode.  That said, the beam code is intended to be python3 
> compatible, so in practice, performing a python3 lint on the Beam code-base 
> is perfectly safe.  The primary risk of doing this is that someone introduces 
> a python-3 only change that breaks python2, but these would largely be syntax 
> errors that would be immediately caught by the unit and integration tests.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to