The new test is failing unfortunately. Please make sure you test with up-to-date matplotlib main on your machine first.

The new test is failing unfortunately. Please make sure you test with up-to-date matplotlib main on your machine first.

Yeah, sure I were going to do that.

@jklymak hey, Why thecodecov/project/tests test are failing?

New contributors don't get all tests run automatically, but that makes codecov fail (because some tests are not being run). Nothing to worry about. I manually started your remaining tests. Thanks!

my pleasure, thank you by your help!

Now why this one test are failing?

If you click through the failing test you can see that it was failing to connect to a server to upload artifacts, which happens sometimes, but is probably not this PRs fault.

That might be

@jklymak Can I left like that? With this test failed?

The tests passed, it was just an upload of artifacts that failed, so I would say its fine.

Ok

@QuLogic I have already solved the issues you pointed out, thanks!

I just merged this branch withmain, and the tests, that previously succeed in passing, now it is getting me these errors

If possible, it'd be preferred if you rebased but it looks like we'll have to squash merge to get rid of the duplicate images anyway, so that might not be necessary.

@QuLogic I rebased it, and I think it worked, the tests are now passing, butcodecov/project/tests no, because as a new contributor, my tests did not run automatically

Your PR has a bunch of extraneous commits, hence the PR cleanliness failure. I'm not sure if a squash merge will get rid of these, if I was fixing this I would do a rebase, drop the extra commits, and then rebase onto latest master. Back up your branch before you do this though so you don't lose anything! Maybe others will have better advice.

(back up means dogit checkout fix-21165; git checkout -b tmp-backup; git checkout fix-21165 and then do the rebasing. If you don't know how to rebase, you will probably need to read a guide online, butgit rebase i 12345678 where12345678 is the last commit ingit log that you didnot author just before the first commit you did author.

@jklymak Apparently solved

How can I fix the fact that my baseline image is duplicated? I delete it in my local repo and push it? Any idea?

You almost had the rebase, but after rebasing, you have to force push up to GitHub. If you pull, you just get back the old commits again and merge them into the history again.

So I see that#21339 also exists, but has not been updated in some time. However, it was also blocked on#21349, so I think cannot merge this until that is fixed; is that right@timhoffm?

@QuLogic So I will have to do the rebase again? If so, could you write a small guide please? I've never done a rebase and the texts I found on internet is a little confusing.

You did it right the first time, you just then undid your work by doing agit pull before force-pushing your changes to GitHub. (Also, please don't ask us to write unique guides for every PR... we have our own guide, listed in the docs, and there are lots of docs online. If you have a specific question about a step, feel free to ask on gitter, thanks for your understanding).

Ok, Sorry by the request, I am learning a lot contributing with open source software.

The tests passed now! Great.

So I see that#21339 also exists, but has not been updated in some time. However, it was also blocked on#21349, so I think cannot merge this until that is fixed; is that right@timhoffm?

@QuLogic a bit confused bythis andthat, but yes, it is preferable if we can solve#21349 before we merge this pull request here.

@timhoffm So what is gonna be the approach in that case?

Right, this PR is fine, but the marginals may change under#21339, so adding the image, and then re-adding it is unecessary churn in our repo. So I'll mark this as depending on another PR and move to draft for now. Thanks for your patience@LucasRicci

@jklymak Just for further information, what is the max period that a PR should be inactivity to be closed?

Just subscribe to upstream issue and when it is merged you can move this one to "ready"

@timhoffm Moving this PR to ready since#21339 was merged

This appears to need a rebase now, though looking again, it appears that this duplicates#21339?

@QuLogic Idk, but if so, I'll close this PR

The test is already covered in#21339 now.@LucasRicci I'm sorry that there has been overlapping work, but the objective of#21339 changed after we realized that the behavior was in fact correct and a test was then added for this.

Anyway, thank you for the PR. Even though this was not merged, your contributions is valuable to bring the discussion forward.