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.

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

increase minimal python version to 3.6 #553

Merged
merged 13 commits into from Jan 29, 2019
Merged

Conversation

renefritze
Copy link
Member

@renefritze renefritze commented Jan 21, 2019

No description provided.

@renefritze
Copy link
Member Author

@renefritze renefritze commented Jan 21, 2019

closes #542
closes #553

I'd ask all @pymor/pymor-devs to look over the changes and see I missed any 3.5 -> 3.6 mentions.

@codecov
Copy link

@codecov codecov bot commented Jan 21, 2019

Codecov Report

Merging #553 into master will decrease coverage by <.01%.
The diff coverage is 38.9%.

Impacted Files Coverage Δ
src/pymor/analyticalproblems/burgers.py 100% <ø> (ø) ⬆️
src/pymor/reductors/bt.py 64.58% <ø> (ø) ⬆️
src/pymor/discretizers/cg.py 90.98% <ø> (ø) ⬆️
src/pymor/reductors/interpolation.py 79.51% <ø> (ø) ⬆️
src/pymor/gui/matplotlib.py 20.37% <0%> (ø) ⬆️
src/pymor/domaindiscretizers/default.py 75.49% <0%> (ø) ⬆️
src/pymor/reductors/residual.py 70.34% <0%> (ø) ⬆️
src/pymor/__init__.py 42.85% <0%> (ø) ⬆️
src/pymor/tools/mpi.py 33.33% <0%> (ø) ⬆️
src/pymor/algorithms/lyapunov.py 94.11% <0%> (ø) ⬆️
... and 71 more

@codecov
Copy link

@codecov codecov bot commented Jan 21, 2019

Codecov Report

Merging #553 into master will not change coverage.
The diff coverage is 39.51%.

Impacted Files Coverage Δ
src/pymor/analyticalproblems/burgers.py 100% <ø> (ø) ⬆️
src/pymor/reductors/bt.py 64.58% <ø> (ø) ⬆️
src/pymor/discretizers/cg.py 90.98% <ø> (ø) ⬆️
src/pymor/grids/tria.py 98.38% <ø> (ø) ⬆️
src/pymor/reductors/interpolation.py 79.51% <ø> (ø) ⬆️
src/pymor/vectorarrays/mpi.py 32.27% <0%> (ø) ⬆️
src/pymor/domaindiscretizers/default.py 75.49% <0%> (ø) ⬆️
src/pymor/reductors/residual.py 70.34% <0%> (ø) ⬆️
src/pymor/gui/matplotlib.py 20.37% <0%> (ø) ⬆️
src/pymor/__init__.py 42.85% <0%> (ø) ⬆️
... and 57 more

.ci/appveyor.yml Show resolved Hide resolved
.ci/appveyor.yml Show resolved Hide resolved
@sdrave
Copy link
Member

@sdrave sdrave commented Jan 22, 2019

LGTM

@sdrave
Copy link
Member

@sdrave sdrave commented Jan 22, 2019

Didn't see that this also includes the f-string conversion. Have to look at this again ..

@sdrave
Copy link
Member

@sdrave sdrave commented Jan 22, 2019

Ok, so I think the automatic f-string conversion is way to conservative. Here you can find some manual conversions I did as a test. I think the result is much better ... -

@renefritze
Copy link
Member Author

@renefritze renefritze commented Jan 22, 2019

I'll look into hacking the converdion limit in pyupgrade then. Unless you want to continue manually changing more 😉

@renefritze
Copy link
Member Author

@renefritze renefritze commented Jan 23, 2019

So pyupgrade does some AST magic and it cannot rewrite cases where the .format input is anything more complex than a value/name. So anything with a function call is skipped atm.

@renefritze
Copy link
Member Author

@renefritze renefritze commented Jan 23, 2019

I've removed that restriction from pyupgrade and added another commit with more f-strings

src/pymor/core/cache.py Outdated Show resolved Hide resolved
src/pymor/core/cache.py Outdated Show resolved Hide resolved
src/pymor/core/interfaces.py Show resolved Hide resolved
@renefritze renefritze mentioned this pull request Jan 26, 2019
@renefritze
Copy link
Member Author

@renefritze renefritze commented Jan 28, 2019

Are you ok with merging this now, @sdrave ?

@sdrave
Copy link
Member

@sdrave sdrave commented Jan 28, 2019

There are still various f-string conversions ahead. Trying to finish this today ...

@renefritze
Copy link
Member Author

@renefritze renefritze commented Jan 28, 2019

If you want I can try to run the try to widen the scope of cases where the tool does the conversion again before you do that manually. Looks like > 100 instances still

@sdrave
Copy link
Member

@sdrave sdrave commented Jan 28, 2019

I just finished the f-string conversions. If tests pass, we can merge IMHO.

@renefritze
Copy link
Member Author

@renefritze renefritze commented Jan 29, 2019

I don't quite understand how https://github.com/pymor/pymor/pull/553/files#diff-291c539e997b6c8ef74ac772372889adL409 could've ever worked? Was that just never evaluated?

@sdrave sdrave added the pr:change Change in existing functionality label Jan 29, 2019
@sdrave sdrave changed the title [CHANGED] increase minimal python version to 3.6 increase minimal python version to 3.6 Jan 29, 2019
@sdrave sdrave added this to the 2019.2 milestone Jan 29, 2019
@renefritze renefritze merged commit 7c42cc0 into master Jan 29, 2019
4 checks passed
@renefritze renefritze deleted the increase_min_py_to_3.6 branch Jan 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:change Change in existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants