GeezerVison

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process.

You can also join us on discourse chat for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

Dear Maintainers,
I have seen how diligently you guys work and how mentally constraining reviewing each line of the code is. Though this issue is a 'good first issue' and not as complex (and may be not as important) as compared to the other bug fixes that the contributors are providing. I would be obliged if you could review my code and give me necessary feedback to improve it as much as possible.

Assuming that I'm talking to an actual human: this PR is currently failing to pass the Linting and MyPy Stubtest checks. The AppVeyor build failure is somewhat expected as AppVeyor has been flaky recently.

Generally, you will want to make sure that all checks pass on any PR that you open to a repository. You need to get at least Linting and the stub test to pass first.

Assuming that I'm talking to an actual human: this PR is currently failing to pass the Linting and MyPy Stubtest checks. The AppVeyor build failure is somewhat expected as AppVeyor has been flaky recently.

Generally, you will want to make sure that all checks pass on any PR that you open to a repository. You need to get at least Linting and the stub test to pass first.

Thank you sir.. on it..

Hi @iccir
I think I have screwed up in some way. Thing is, I have done a pull request and merged the code, so instead of just 1 commit, there is 11 commits and also some failed tests (of other developers). I have checked my tests (linting and all) and everything is successful. My question is: will everything work or do I have to open a new pull request?
Sorry for any trouble I have caused.

I think a rebase should sort that out. Assuming you have set the upstream remote as described here, try this:

Make a backup branch in case something else goes wrong

git branch my-backup-branch 

Rebase on the upstream branch

git fetch upstream
git rebase upstream/main

git push will then fail, but you can

git push --force-with-lease

Hey, it happens - git can be tricky :)

It looks like you made two commits:
c278400
a14df4f

I'd do something like the following:

git diff \
    c278400dee9b6758ce74f0eab9b5cbc085b595ee^ \
    a14df4f320b0b13ebdf7fc34cf4d356e5f0f67ea > \
    /someplace/safe/changes.patch

Then I'd follow @rcomer's instructions above. You can always reapply the patch with:

git apply /someplace/safe/changes.patch

Hi @rcomer @iccir
Thank you for being so kind and helpful. I have solved the issue with the merge request.
However, as shown, there is a failing test with Mac OS and I don't have a MAC so I really don't know what the issue is.

If you get a test failure that you don't understand, there are several possibilities:

1. The test failure is caused by a specific version of Python. For example, we could see Python 3.12 failing across several different operating systems.
2. The test failure is caused by a specific platform. We could see all macOS builders fail, for example.
3. A test failure on a specific version of Python and a specific platform. You would see a single test failure, as in this case.

More commonly, however, #3 can indicate that something is wrong with the Continuous Integration (CI) system.

You can often look at other pull requests to see if they are having similar issues. Python 3.12 on macOS 15 has been failing all day - here's another example with the same error.

So, it's probably not an error in your code!

If you get a test failure that you don't understand, there are several possibilities:

1. The test failure is caused by a specific version of Python. For example, we could see Python 3.12 failing across several different operating systems.
2. The test failure is caused by a specific platform. We could see all macOS builders fail, for example.
3. A test failure on a specific version of Python and a specific platform. You would see a single test failure, as in this case.

More commonly, however, #3 can indicate that something is wrong with the Continuous Integration (CI) system.

You can often look at other pull requests to see if they are having similar issues. Python 3.12 on macOS 15 has been failing all day - here's another example with the same error.

So, it's probably not an error in your code!

I also have the same view as you. It is showing that the test is failing only in macos-15 (macos26 is running successfully). Also, I have tested the code locally as well, so I'm pretty confident about it. My request is if you could review my code so as to ensure if there are other major issues or this code can be merged to the repo..

Hi @rcomer
Sorry to disturb you like this. But is it possible for you to review my code?

@timhoffm
I have made the changes as mentioned in the review. Please check if the changes made are correct and again thank you for your valuable feedback.

Hi @timhoffm
Could you please check if there are any issues? If not, then could you please approve my PR?

Hi @timhoffm
Could you please review the code again? As you mentioned instead of frozensets, I used sets but inside a list (since a set is unhashable and using it inside another set will give an error). Do mention if there are any more improvements needed..

The test structure is good now.

The copilot comment is a valid point. What do we do about it? #32437 (comment)

The issue is that my fix is quite incomplete in the sense that if instead of standard notations, the user uses scientific notations (The tick_params call leaves the OffsetBox text fully visible on inner axes). Also, since we left aside labeltop and labelright visibility, it will popup as well.

So, it's your call Captain.

We should not ship an incomplete implementation. We actually have the semantics sort of available via _label_outer_xaxis() / _label_outer_yaxis().

Please check: That functionality is coded for regular grids and is not directly suitable for mosaic. Can one split this into (a) detecting whether an axis is "outer" and (b) applying the needed config to the axis. (a) is different for grids and mosaic; (b) may be reusable for both cases.

Hi @timhoffm
I have added changes you recommended. At the same time, I did a lite review and another issue came up with polar coordinates. I have fixed that as well.
So if you can review this PR?

Thank you @timhoffm
Actually this is my first PR and I don't know much about the workflows.. so now do I have to wait for this PR to be merged automatically? Hope I am not rude with this question.

Hi @nillohitroy. Code changes require two approving reviews so you'll need one more approval before this PR can be merged (which is handled by a maintainer). Your contribution is appreciated, but keep in mind that review capacity can sometimes be limited.

Hi @lucaznch
Thank you for taking out your precious time and answering my question. I appreciate the work, maintainers are doing and I can understand how much time and energy it requires to review all these code. So I can patiently wait for it.