-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Fix subplot docs #29471
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
Fix subplot docs #29471
Conversation
lib/matplotlib/pyplot.py
Outdated
@@ -1450,15 +1450,13 @@ def subplot(*args, **kwargs) -> Axes: | |||
|
|||
Notes | |||
----- | |||
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:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay I'll create a new token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
d23af0d
to
43e875b
Compare
# 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted above, this is under-indented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I tried making changes.. I hope the checks passes this time 😭
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried adding 'axes.remove' in other still checks failed, am i suppose to add '.. automethod:: matplotlib.axes.Axes.remove' instead ??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rcomer Thankyou so much for the proper explanation & guidance ..I'll work around it😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I installed pre-commit hooks & pushed the changes.. you can review it once & ask me for any further changes if required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something wrong in the PR ?? I edited the white space thing in documentation still the error ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing wrong, just sometimes things fall off the to-do list. Thanks for the reminder, and for your work on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😄
…471-on-v3.10.x Backport PR #29471 on branch v3.10.x (Fix subplot docs)
PR summary
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.PR checklist
subplot
behavior is not same as the doc reads in 3.10(stable) #29447" is in the body of the PR description to issue