minimal set of changes to pass pylint 2.11.1 tests#2401
Conversation
This disables a lot of changes to allow compatibility with Python 2. These will be re-enabled on dev with a view to dropping Python 2 support in future releases.
maxpietsch
left a comment
maxpietsch
left a comment
There was a problem hiding this comment.
Looks all good to me, pylint passes, script tests pass except some 5ttgen tests because I don't have freesurfer installed.
Lestropie
left a comment
Lestropie
left a comment
There was a problem hiding this comment.
Code looks good to me. Max's tests should suffice, the FreeSurfer-dependent parts are unlikely to suddenly expose a Python failure.
Hopefully it'll simplify the merge to dev, which I don't anticipate will be straightforward...
Not looking forward to it. I need to do a comprehensive read of all of the Python code; would actually like to implement code checks to identify and remove various code that's there for Python2 compatibility (e.g. make sure distutils.spawn.find_executable() should no longer be used). But these comde comments should actually help a little. Also need to decide on philosopohy around how things like string data and encoding/decoding will be managed throughout.
|
Thanks @maxpietsch. I've also run the scripts tests on my system, using both Python 2 & 3. I get failures with I'll wait till @Lestropie gives the OK from his side, just on the off-chance there are still concerns. |
3 participants
Follows on from discussion on #2392. Pushing to a new branch / pull request to preserve the changes originally submitted by @maxpietsch, many of which may be appropriate for
dev.This PR disables a lot of warnings to allow compatibility with Python 2. These will be re-enabled on
devwith a view to dropping Python 2 support in future releases. Minor fixes have been applied where that could be done safely without affecting Python 2 compatibility, and where the changes would be expected to carry over todevanyway.Most of the changes boil down to disabling the warnings at the file level, along with a note to deal with them in
dev. The rationale is to help identify these when merging todev- it's otherwise difficult to work out which warnings need to be addressed and which are expected to remain disabled, when these warnings are added in the globaldisableline in thepylint.rc, or on a line-by-line basis. Hopefully it'll simplify the merge todev, which I don't anticipate will be straightforward...