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.

Sign up for GitHub

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

Jump to bottom

[19.05] Run futurize for python2 cheetah templates, fix scope issue for py3 comprehensions #7867

Merged
merged 10 commits into from
May 8, 2019
Merged

[19.05] Run futurize for python2 cheetah templates, fix scope issue for py3 comprehensions #7867

merged 10 commits into from
May 8, 2019

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented May 1, 2019

I think this should cover the python 2/3 errors I have seen so far

@mvdbeek mvdbeek force-pushed the fix_scope_issue_for_py3_list_comprehensions branch from 46f8641 to 4f7c193 Compare May 1, 2019 17:40
@galaxybot galaxybot added this to the 19.09 milestone May 1, 2019
@mvdbeek
Copy link
Member Author

mvdbeek commented May 1, 2019

Hah, turns out dict, set and generator comprehensions all have that problem on python 2, so I guess we don't need to fix that.

@mvdbeek mvdbeek changed the title Run futurize for python2 cheetah templates, fix scope issue for py3 comprehension and generator statements Run futurize for python2 cheetah templates, fix scope issue for py3 comprehensions May 1, 2019
@mvdbeek mvdbeek added area/python3 Specific to Python 3 kind/enhancement and removed triage labels May 1, 2019
@mvdbeek mvdbeek force-pushed the fix_scope_issue_for_py3_list_comprehensions branch from 7f53d90 to d3821c0 Compare May 1, 2019 18:28
@mvdbeek mvdbeek mentioned this pull request May 1, 2019
Closed
29 tasks
@mvdbeek mvdbeek force-pushed the fix_scope_issue_for_py3_list_comprehensions branch 4 times, most recently from 832b241 to 02c4c46 Compare May 2, 2019 12:40
@mvdbeek mvdbeek marked this pull request as ready for review May 2, 2019 13:22
nsoranzo
nsoranzo reviewed May 2, 2019
View reviewed changes
Copy link
Member

@nsoranzo nsoranzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to parse python_template_version with packaging.version.parse() instead of casting with float(), see comments inline.

test/unit/tools/test_evaluation.py Outdated Show resolved Hide resolved
test/unit/tools/test_evaluation.py Outdated Show resolved Hide resolved
lib/galaxy/tools/__init__.py Outdated Show resolved Hide resolved
lib/galaxy/tools/__init__.py Outdated Show resolved Hide resolved
lib/galaxy/tools/actions/__init__.py Outdated Show resolved Hide resolved
lib/galaxy/util/template.py Show resolved Hide resolved
lib/galaxy/util/template.py Outdated Show resolved Hide resolved
lib/galaxy/util/template.py Outdated Show resolved Hide resolved
test/unit/tools/test_fill_template.py Outdated Show resolved Hide resolved
test/unit/tools/test_fill_template.py Outdated Show resolved Hide resolved
@mvdbeek mvdbeek force-pushed the fix_scope_issue_for_py3_list_comprehensions branch from 205b4a6 to 5cafb90 Compare May 2, 2019 17:48
@mvdbeek mvdbeek force-pushed the fix_scope_issue_for_py3_list_comprehensions branch from 040f8e6 to 3d56230 Compare May 7, 2019 14:27
@mvdbeek mvdbeek changed the base branch from dev to release_19.05 May 7, 2019 14:29
@mvdbeek mvdbeek changed the title Run futurize for python2 cheetah templates, fix scope issue for py3 comprehensions [19.05] Run futurize for python2 cheetah templates, fix scope issue for py3 comprehensions May 7, 2019
@mvdbeek mvdbeek requested a review from nsoranzo May 7, 2019 14:29
mvdbeek added 6 commits May 7, 2019 18:20
@mvdbeek
Make variables that have not been found global
9ff9d55 
This will allow tools to work that access variables from
the enclosing scope in a list/dict/tuple comprehension.
@mvdbeek
Add test that unfortunately shows that the global variable isn't updated
85e13d2 
Now if we could figure out the exact position in the template source
where a NotFound error is raised we could replace `$variable`
with `variable` and add `#variable = $variable` just before the failing
statement.
@mvdbeek
Run futurize on Compiled Cheetah Module
5ef41e7
@mvdbeek
Work around list comprehension scope issues
0062bce
@mvdbeek
Non-list comprehensions can't refer to non-iteration variables from s…
510f23f 
…urrounding scope

While fixable this seems to not work on python 2 either, so we will not
attempt to fix it.
@mvdbeek
Only run python 3 fixers if profile version < 19.05
d25a5dd 
or python_template_version < 3.
nsoranzo and others added 2 commits May 7, 2019 18:20
@nsoranzo @mvdbeek
Use packaging.version.parse() instead of float() for version compar…
1d1f727 
…isons
@mvdbeek
Use lib2to3 RefactoringTool instead of external process
a05d24c 
We use a list of fixers from past.translation. This is just a
list of strings pointing to fixers to import. Currently this is
```
['libfuturize.fixes.fix_absolute_import',
 'libfuturize.fixes.fix_print_with_import',
 'libfuturize.fixes.fix_raise',
 'libfuturize.fixes.fix_next_call',
 'lib2to3.fixes.fix_reduce',
 'lib2to3.fixes.fix_sys_exc',
 'lib2to3.fixes.fix_throw',
 'lib2to3.fixes.fix_tuple_params',
 'lib2to3.fixes.fix_exec',
 'lib2to3.fixes.fix_standarderror',
 'lib2to3.fixes.fix_apply',
 'lib2to3.fixes.fix_paren',
 'lib2to3.fixes.fix_types',
 'lib2to3.fixes.fix_methodattrs',
 'lib2to3.fixes.fix_renames',
 'lib2to3.fixes.fix_ws_comma',
 'lib2to3.fixes.fix_repr',
 'lib2to3.fixes.fix_intern',
 'lib2to3.fixes.fix_numliterals',
 'lib2to3.fixes.fix_funcattrs',
 'lib2to3.fixes.fix_ne',
 'lib2to3.fixes.fix_exitfunc',
 'lib2to3.fixes.fix_has_key',
 'lib2to3.fixes.fix_isinstance',
 'lib2to3.fixes.fix_idioms',
 'lib2to3.fixes.fix_xreadlines',
 'lib2to3.fixes.fix_except',
 'libfuturize.fixes.fix_execfile',
 'libfuturize.fixes.fix_future_standard_library',
 'libfuturize.fixes.fix_metaclass',
 'libfuturize.fixes.fix_future_builtins',
 'libpasteurize.fixes.fix_newstyle',
 'libfuturize.fixes.fix_basestring',
 'libfuturize.fixes.fix_future_standard_library_urllib',
 'libfuturize.fixes.fix_xrange_with_import',
 'libfuturize.fixes.fix_cmp',
 'libfuturize.fixes.fix_unicode_keep_u',
 'libfuturize.fixes.fix_division_safe',
 'libfuturize.fixes.fix_object',
 'lib2to3.fixes.fix_input',
 'lib2to3.fixes.fix_operator',
 'lib2to3.fixes.fix_zip',
 'lib2to3.fixes.fix_dict',
 'lib2to3.fixes.fix_next',
 'lib2to3.fixes.fix_filter',
 'lib2to3.fixes.fix_raw_input',
 'lib2to3.fixes.fix_getcwdu',
 'lib2to3.fixes.fix_itertools_imports',
 'lib2to3.fixes.fix_nonzero',
 'lib2to3.fixes.fix_itertools',
 'lib2to3.fixes.fix_long',
 'lib2to3.fixes.fix_map']
```
And then we drop pasterurize, which we don't need for our purposes.
@mvdbeek mvdbeek force-pushed the fix_scope_issue_for_py3_list_comprehensions branch from 3d56230 to a05d24c Compare May 7, 2019 16:21
@galaxyproject galaxyproject deleted a comment from mvdbeek May 7, 2019
@mvdbeek mvdbeek mentioned this pull request May 8, 2019
Closed
@mvdbeek mvdbeek force-pushed the fix_scope_issue_for_py3_list_comprehensions branch from f16cf9f to ccea1ee Compare May 8, 2019 11:41
@mvdbeek
Copy link
Member Author

mvdbeek commented May 8, 2019

@galaxybot test py3

@martenson martenson merged commit 5dec2a3 into galaxyproject:release_19.05 May 8, 2019
@martenson
Copy link
Member

merged forward

@mvdbeek mvdbeek mentioned this pull request May 8, 2019
Merged
@martenson martenson deleted the fix_scope_issue_for_py3_list_comprehensions branch May 22, 2019 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmchilton jmchilton jmchilton approved these changes

@nsoranzo nsoranzo Awaiting requested review from nsoranzo

Assignees
No one assigned
Labels
area/python3 Specific to Python 3 kind/enhancement
Projects
None yet
Milestone
19.09
Development

Successfully merging this pull request may close these issues.
5 participants
@mvdbeek @martenson @jmchilton @nsoranzo @galaxybot