FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix subplot docs by Khushikela29 · Pull Request #29471 · matplotlib/matplotlib · GitHub

Repository navigation

Fix subplot docs - #29471

Merged
rcomer merged 4 commits into
matplotlib:mainfrom
Khushikela29:fix-subplot-docs
Jan 24, 2025
Merged

rcomer merged 4 commits into
matplotlib:mainfrom
Khushikela29:fix-subplot-docs

Conversation

Khushikela29 commented Jan 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR summary

  • This PR fixes [Doc]: subplot behavior is not same as the doc reads in 3.10(stable) #29447 i.e. updates the docstring for 'matplotlib.pyplot.subplot' to reflect the current behavior in Matplotlib 3.10.
  • The current documentation for 'matplotlib.pyplot.subplot' incorrectly states that creating a new 'Axes' will delete any preexisting 'Axes' that overlaps with it.
  • The PR updates the docstring to describe the current behavior, ensuring that the documentation aligns with the functionality.

PR checklist

github-actions Bot added topic: pyplot API Documentation: examples files in galleries/examples Documentation: devdocs files in doc/devel labels Jan 14, 2025

QuLogic commented Jan 15, 2025

Copy link
Copy Markdown
Member

It appears you have accidentally included your commits from #29433 and #29463; please rebase on the current main to remove them.

Comment thread lib/matplotlib/pyplot.py Outdated
-----
Creating a new Axes will delete any preexisting Axes that
overlaps with it beyond sharing a boundary::
Creating a new Axes will not delete any preexisting Axes, even if they overlap::

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The note should be rewritten. Replace this up to line 1463 by

.. versionchanged:: 3.8
   In versions prior to 3.8, any preexisting Axes that overlap with the new Axes
   beyond sharing a boundary was deleted. Deletion does not happen in more
   recent versions anymore. Use `.Axes.remove` explicitly if needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

okay sure, I'll do that.
one ques: am I suppose to rebase everytime I commit ? I dont know how last commit was included in this one & same happened when i was doing the last commit also.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No you do not have to rebase on every commit. I suspect you may have created this feature branch from the other feature branch and not from main. You should always checkout main first and then create the new feature branch from there. Otherwise, you get a coupling between the feature branches.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I agree that i didnt do it while raising 2nd PR but I did followed this during this PR. I dont know whats wrong, I'll try to do this in next PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

also I wanted to share what I've changed so that I open PR without mistakes, how do I get it verified first ? & than commit changes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It seems like you logged in to GitHub on your command-line Git client with a reduced-permission token. I'm not sure how you did that exactly (maybe using GitHub Desktop?), but you would have to create a new token with more permissions. Or consider using SSH authentication instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

okay I'll create a new token

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this is the last commit i made on this project... here is the screenshot what should i do ? should I just merge the conflict without resolving or I am suppose to make changes.. I fear that it can impact the last commit i made

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@Khushikela29 you have changed a couple of lines. If you are afraid of losing those changes, just copy them to a text file somewhere so you can paste them back if needed.

If it were me in this situation, I would do

git fetch upstream
git checkout fix-subplot-docs
git reset --hard upstream/main

This will reset the branch to upstream/main giving you a clean slate. All your previous commits will be erased, including your changes - again you may want to copy your changes into a temporary file.

I would then make your small change.

I would then

git commit -a -m "DOC: your change desk"
git push origin fix-subplot-docs --force

You need to --force to overwrite your previous changes.

That should update this pull request with just one commit that has been changed from upstream/main.

Khushikela29 Jan 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@jklymak Thankyou so much.. I was struggling from 2 days on how do I reset the code.. I mean thankyou, it was great learning & looking forward for more like these.
Still some of the checks didnt pass in the PR, i'll look into it maybe some docstring is not written correctly.

github-actions Bot removed Documentation: examples files in galleries/examples Documentation: devdocs files in doc/devel labels Jan 18, 2025
Comment thread lib/matplotlib/pyplot.py
# with 2 rows and 1 column. Since this subplot will overlap the
# first, the plot (and its Axes) previously created, will be removed
plt.subplot(211)
.. versionchanged:: 3.8

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

As noted above, this is under-indented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

yes, I tried making changes.. I hope the checks passes this time 😭

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Now you need to click through the CircleCI job. I usually download the raw log and search for "Warning" to see why the doc build failed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There is also a section below the failed build output called "Extract possible build errors and warnings". It seems you are trying to link to Axes.remove, which is not in the docs. Perhaps it should be added under "Other" here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I tried adding 'axes.remove' in other still checks failed, am i suppose to add '.. automethod:: matplotlib.axes.Axes.remove' instead ??

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@rcomer Thankyou so much for the proper explanation & guidance ..I'll work around it😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I installed pre-commit hooks & pushed the changes.. you can review it once & ask me for any further changes if required.

Khushikela29 Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Something wrong in the PR ?? I edited the white space thing in documentation still the error ?

rcomer Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nothing wrong, just sometimes things fall off the to-do list. Thanks for the reminder, and for your work on this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No worries :)
I actually wanted to thankyou guys for being patient in this PR, I really appreciate your guidance & patience on this PR.. looking forward for more meaningful contributions & learning ahead!!
Thankyouu 😄

github-actions Bot added the Documentation: API files in lib/ and doc/api label Jan 19, 2025
rcomer added this to the v3.10.1 milestone Jan 24, 2025
rcomer merged commit 5220ebe into matplotlib:main Jan 24, 2025
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 24, 2025
rcomer added a commit that referenced this pull request Jan 24, 2025
…471-on-v3.10.x

Backport PR #29471 on branch v3.10.x (Fix subplot docs)
ksunden mentioned this pull request Mar 3, 2025
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Doc]: subplot behavior is not same as the doc reads in 3.10(stable)

5 participants


Back | FazBrowse Home | New Git URL