Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Backport PRs #12154, #12294, #12297, #12316, #13159 & #13205 to fix multiple tests issues#13181

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
QuLogic merged 16 commits intomatplotlib:v2.2.xfromArchangeGabriel:auto-backport-of-pr-12154-on-v2.2.x
Jan 22, 2019
Merged

Backport PRs #12154, #12294, #12297, #12316, #13159 & #13205 to fix multiple tests issues#13181

QuLogic merged 16 commits intomatplotlib:v2.2.xfromArchangeGabriel:auto-backport-of-pr-12154-on-v2.2.x
Jan 22, 2019

Conversation

ArchangeGabriel
Copy link
Contributor

Once merged, a follow-up PR should revertd7e3789 as was done in#13159 for v3.0.x.

My only fear is that bumpingpytest requirement could be seen as an issue (based on my interpretation on#13159 (comment)).

@ArchangeGabriel
Copy link
ContributorAuthor

appveyor CI fails because ofpytest-rerunfailures 6.0. I guess this is a missing constraints upadte ind7e3789 (travis was changed but not appveyor). Should I add it here? And then revert in the follow-up PR?

Travis CIs fail withsh: 0: Can't open /etc/init.d/xvfb, not sure if related.

@tacaswelltacaswell added this to thev2.2.4 milestoneJan 14, 2019
@tacaswell
Copy link
Member

My concern about bumping the pytest version requirement on the 2.2.x branch is that we are going to be caught in this trap where either the slower distributions will not have a new enough version of pytest or the faster distributions will not have an old enough version (pytest 3.6 is only from May 2018).

Could you do all of the backporting / reverting in this PR? I think that makes more sense than a series of PRs.

@ArchangeGabriel
Copy link
ContributorAuthor

Sure, I’ll push the revert in a second commit.

Are you aware of distributions still updating matplotlib (even if we are talking patch version here) but not pytest?

@ArchangeGabriel
Copy link
ContributorAuthor

Pushed. Note that in#12879 (comment),@timhoffm also stated that fixing this in 2.2.x might not be wanted, not sure if this is for the same reason.

@ArchangeGabriel
Copy link
ContributorAuthor

AppVeyor is now failing with numerousE TypeError: cannot concatenate 'str' and 'MarkDecorator' objects, which I already started to investigate in#13182.

@ArchangeGabriel
Copy link
ContributorAuthor

Errors fromhttps://ci.appveyor.com/project/matplotlib/matplotlib/builds/21606888/job/6jl1g3mg6jn1j2ol?fullLog=true are gone with the latest commit (btw, I’m not sure about the coding style in theps test). However, theTypeError: cannot concatenate 'str' and 'MarkDecorator' objects failures (1225 over 1229 FAILURES) are still there, as well as the four remaining ones,TypeError: coercing to Unicode: need string or buffer, MarkDecorator found.

I’ll keep investigating, but that is already going beyond my knowledge, so help would be appreciated.

@ArchangeGabriel
Copy link
ContributorAuthor

I guess the solution is to be deduced fromhttps://docs.pytest.org/en/latest/mark.html#marker-revamp-and-iteration, but having never manipulated such things I’m a bit lost…

Backporting and testing is easy, fixing is something else. :p

@ArchangeGabriel
Copy link
ContributorAuthor

pytest-dev/pytest#3576 might also be related.

@tacaswell
Copy link
Member

attn@sandrotosi is this going to be a problem of debian?

@ArchangeGabriel
Copy link
ContributorAuthor

(Might be worst noting that in the latestv2.2.x commit, all tests failing here are skipped)

Also I still don’t understand the Travis failure.

@tacaswell
Copy link
Member

This looks like the thing we use to have an xserver on travis stopped working...

@tacaswell
Copy link
Member

@ArchangeGabriel Be aware I added some commits (it seemed faster to just make the edits via the GH ui than to ask you to make the changes ), hope you do not mind.

@tacaswell
Copy link
Member

ok, that seems to have gotten the python3 test running again, but py27 is failing inside of pytest startup...

@ArchangeGabriel
Copy link
ContributorAuthor

@tacaswell Yes I’ve seen, no issue with that, you are very welcome! ;)

So Python 3.4 is working, Python 3.6 & 3.7 are failing with the same error as in#13182. AppVeyor is not working because the lack of tools to compare images makes it try to skip 1229 tests, and the skipping code is what is failing with newer pytest on Py2.

Regarding Debian, it should not be an issue:Stretch hasmatplotlib 2.0.0, whileBuster hasmatplotlib2 2.2.3 butpytest 3.10.1 (https://packages.debian.org/source/matplotlib andhttps://packages.debian.org/source/pytest).

@ArchangeGabriel
Copy link
ContributorAuthor

(I’m investigating the py2 issue, already foundpytest-dev/pytest#3889, so likely have to go an higher pytest version)

@ArchangeGabriel
Copy link
ContributorAuthor

ArchangeGabriel commentedJan 16, 2019
edited
Loading

Yup, fixed in 3.6.1:pytest-dev/pytest@b5a94d8

I’ll push a small commit to update to this version at least.

@ArchangeGabriel
Copy link
ContributorAuthor

OK, so now need to re-pin downrerunfailures. I guess<6 will do, checking and pushing.

@ArchangeGabriel
Copy link
ContributorAuthor

Wow, py2 actually passed in Travis. \o/ I’ll have a look in version differences between the different passing/failing builds, to see whether they are some more changes we should catch or if there is a real issue in pytest.

@ArchangeGabriel
Copy link
ContributorAuthor

Small note: the working Py3.4 TravisCI is usingpytest 3.8.2, while the failing Py3.6/3.7 are usingpytest 4.1.1. Looking at thepytest changelog, I don’t see anything outstanding, but I would bet onpytest 4.1 being the culprit (and since I did not see anything relevant in the changes, I would say it’s apytest bug).

@ArchangeGabriel
Copy link
ContributorAuthor

OK, got it. We need to backport#12316 (mainlydbac64f and46d3860, but others one are interesting too). I’ll do it tomorrow if needed, but if someone with appropriate rights can runmeseeksdev in the meantime so that we can at best win some time and at worst give me the appropriate way of backporting a multi-commit PR, that would be very welcomed.

@ArchangeGabriel
Copy link
ContributorAuthor

CI went OK as expected. I’ve fixed mypytest>=3.6.1 commit, and also backported#12294 (but the backport was automatic, so I didn’t take care of the commit message).

I think this PR is now up for review/comments, especially if you want me to change some commit messages or coding style for manual changes.

@ArchangeGabriel
Copy link
ContributorAuthor

@NelleV Not sure why you cycled, the CI gave the same result as just before. MacOS on Travis is broken trying to compile half of the universe before being able to install, so…

@QuLogic
Copy link
Member

It might be necessary to backport#12615 to fix macOS builds.

@QuLogic
Copy link
Member

Though it seems to be ffmpeg that fails, so maybe not.

It seems to be timing out the build (because it is falling back to building ffmpeg from scratch?!).
@tacaswell
Copy link
Member

we are not seeing this issue in master because we are trying to install a non-existing tap (Error: No available formula with the name "font-wenquanyi-zen-hei") so this whole line is not run and we do not try to install ffmpeg.

@NelleV
Copy link
Member

@ArchangeGabriel I cycle when patches merged in the branch may (or may not) fix the test issues. I don't necessarly try to deeply look at the whether the failed tests on the branch correspond to the tests fixed in a previous patch: it would take too much of our time to manually check this while restarting the CI only takes machine time.

@ArchangeGabriel
Copy link
ContributorAuthor

@NelleV I see, makes sense. :)

So we are now passing with macOS too. What are the next steps for this to get merged?

Copy link
Member

@NelleVNelleV left a comment

Choose a reason for hiding this comment

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

Thanks!

@NelleV
Copy link
Member

@ArchangeGabriel You bug people until they review your pull request. The next core dev' that approves can merge it, so hopefully it should be fast!

@sandrotosi
Copy link
Contributor

@tacaswell this should be fine for Debian - thanks for checking!

@sandrotosi
Copy link
Contributor

also, small notice, we're about to start the freeze phase (ie no more new packages) for the next stable Debian release, so if you're planning on releasing mpl 2.x and 3.x this would be a good time :)

@NelleV
Copy link
Member

😬

@ArchangeGabrielArchangeGabriel changed the titleBackport PR #12154: Avoid triggering deprecation warnings with pytest 3.8.Backport PRs #12154, #12294, #12297, #12316, #13159 & #13205 to fix multiple tests issuesJan 22, 2019
@ArchangeGabriel
Copy link
ContributorAuthor

I’ve updated the title to reflect more correctly what’s inside. ;)

@QuLogicQuLogic merged commit4a5064c intomatplotlib:v2.2.xJan 22, 2019
@QuLogic
Copy link
Member

Congrats on getting your first PR into Matplotlib!

@ArchangeGabriel
Copy link
ContributorAuthor

Thanks! :)

@tacaswell
Copy link
Member

@sandrotosi may try to do bug fix releases for 3.0.x and 2.2.x by end end of January.

@shoyer
Copy link

Was this ever backported into 3.0.x?

I see the changes on the 2.2.x branch but not 3.0.x, e.g., in
https://github.com/matplotlib/matplotlib/blob/v3.0.x/lib/matplotlib/testing/conftest.py

@tacaswell
Copy link
Member

No, we missed it on accident (see#12154 (comment) ).

Given that we are going to do a 3.1 rc, mixed on if we should do a 3.0.4 to put this in...

arokem added a commit to arokem/nitime that referenced this pull requestJun 25, 2019
Maybe will fix this error:https://travis-ci.org/nipy/nitime/jobs/550415121Not sure, but this suggests it might help:matplotlib/matplotlib#13181
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@NelleVNelleVNelleV approved these changes

@QuLogicQuLogicQuLogic approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v2.2.4
Development

Successfully merging this pull request may close these issues.

9 participants
@ArchangeGabriel@tacaswell@QuLogic@NelleV@sandrotosi@shoyer@dstansby@jklymak@anntzer

[8]ページ先頭

©2009-2025 Movatter.jp