Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Removal of deprecations for Contour#26907
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
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.
You can also join uson gitter for real-time discussion.
For details on testing, writing docs, and our review process, please seethe developer guide
We strive to be a welcoming and open project. Please follow ourCode of Conduct.
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 code that has been removed is not needed in the removal note, please refer to the deprecation note, which can be copy/pasted and updated to reflect the removal rather than deprecation:
matplotlib/doc/api/prev_api_changes/api_changes_3.7.0/deprecations.rst
Lines 113 to 127 in6028039
``contour.ClabelText`` and ``ContourLabeler.set_label_props`` | |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | |
... are deprecated. | |
Use ``Text(..., transform_rotates_text=True)`` as a replacement for | |
``contour.ClabelText(...)`` and ``text.set(text=text, color=color, | |
fontproperties=labeler.labelFontProps, clip_box=labeler.axes.bbox)`` as a | |
replacement for the ``ContourLabeler.set_label_props(label, text, color)``. | |
``ContourLabeler`` attributes | |
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | |
The ``labelFontProps``, ``labelFontSizeList``, and ``labelTextsList`` | |
attributes of `.ContourLabeler` have been deprecated. Use the ``labelTexts`` | |
attribute and the font properties of the corresponding text objects instead. |
Additionally, please rename the file with the PR number rather than00001
.
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.
Do you feel comfortable squashing the commits to avoid adding and removing the changelog file?
Other than that, just a bit of clean up of the changelog file left
Removal change template | ||
~~~~~~~~~~~~~~~~~~~~~~~ | ||
Enter description of methods/classes removed 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.
Removal change template | |
~~~~~~~~~~~~~~~~~~~~~~~ | |
Enter description of methods/classes removed here.... |
Please rename file with PR number and your initials i.e. "99999-ABC.rst" | ||
and ``git add`` the new file. |
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.
Please rename file with PR number and your initials i.e. "99999-ABC.rst" | |
and ``git add`` the new file. |
This is missing the body of that change note
Thanks for the feedback, I'll try to figure it out. This is my first time and I'm not familiar with squashing, but I'll try. |
Uh oh!
There was an error while loading.Please reload this page.
We have some docs on squashing via https://matplotlib.org/stable/devel/development_workflow.html#rewrite-commit-history |
Also remove from the corresponding |
Thanks, I think I figured it out. |
It shows one test failing Mypy. I'm having trouble understanding what the issue is, any suggestions on what I need to do to correct? |
I agree the Mypy message could be clearer, but Ithink it’s just that you have removed some methods from the |
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.
Just some lint things, if you can sqaush again, that might be ideal as well, though we can also just do that on merge
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
The ``labelFontProps``, ``labelFontSizeList``, and ``labelTextsList`` | ||
attributes of `.ContourLabeler` have been removed. Use the ``labelTexts`` | ||
attribute and the font properties of the corresponding text objects 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.
attribute and the font properties of the corresponding text objects instead. | |
attribute and the font properties of the corresponding text objects instead. | |
lib/matplotlib/contour.py Outdated
@@ -1909,4 +1871,4 @@ def _initialize_x_y(self, z): | |||
<https://en.wikipedia.org/wiki/Marching_squares>`_ algorithm to | |||
compute contour locations. More information can be found in | |||
`ContourPy documentation <https://contourpy.readthedocs.io>`_. | |||
""" % _docstring.interpd.params) | |||
""" % _docstring.interpd.params) |
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.
"""%_docstring.interpd.params) | |
"""%_docstring.interpd.params) | |
QuLogic commentedSep 27, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
stubtest is still complaining; the entries corresponding to the removed ones need to be removed from the |
I'm completely lost at this point. I don't understand where all the problems are coming from. |
You seem to have done the (roughly) same thing you did previously regarding rebasing. If you only have the last commit, it looks correct (though I would want CI to run to be sure). One issue is that main has also updated the Unfortunately that is one big long line that makes it hard to tell what the differences are: It was edited in#26874, which removed the The following should resolve it: git rebase --onto upstream/main HEAD~1# Resolve the conflict noted above, leaving that line in once, the git `>>>>`/`====`/`<<<<` lines removed, etc and both entries to be removed left out$EDITOR doc/api/artist_api.rstgit add doc/api/artist_api.rstgit rebase --continue# Edit commit messagegit push --force-with-lease If you'd rather we do it, I'm willing, but that is what I believe is needed. |
Thanks, I'll give it one more try. Thanks for the patience. |
QuLogic commentedOct 3, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@DHClimber it appears something messed up here, as you have only a single newline added to one file now. I think the correct commit isf448c0f. You can re-fetch that with (assuming you have the
Then you can reset your branch to it:
and push to replace it here:
|
Thank you, I have been out of town traveling. I'll try to fix now. |
It seems like correct commit now, but I'm confused what is causing automated tests to fail. If anyone can provide a hint I'm happy to try and correct it. |
It looks like you have some conflicts - it may be that something didn't go right on your rebase. Could you try checking again? Here's relevant documentation, hope it helps:https://matplotlib.org/devdocs/devel/development_workflow.html#manage-commit-history If you need extra help let us know. |
DHClimber commentedOct 10, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sorry, I've tried everything I can think of but nothing is making sense. I can't understand where the conflicts are coming from, I'm just shooting in the dark now. artist_api.rst error is complaining my branch doesn't match "main", but I don't understand why that is an error when the file needed to be updated? |
I think I should have cleaned that up, I actually had a branch from the first time I advised a rebase sitting on one of my machines that I had taken care of the conflicts on, so I just rebased that again and pushed it up |
Thanks! I was so confused, didn't even know that was a possibility. |
Can you update the commit message? It currently is an incomplete statement: 'removal of deprecated'. It would be better if it matched this PR title, for example. |
fix pyifix pyi
Sorry about that, should be corrected now. |
@@ -16,7 +16,7 @@ from collections.abc import Callable, Iterable, Sequence | |||
from typing import Literal | |||
from .typing import ColorType | |||
class ClabelText(Text): ... | |||
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.
Remove this line instead.
Just following up, is there anything else that needs to be done here? |
We require two approvals before merges, so this is waiting for a second reviewer. |
Uh oh!
There was an error while loading.Please reload this page.
PR summary
This change is related to[MNT]: Remove 3.7-deprecated API
#26865
This pull request removes deprecated classes and methods from contour.py and associated contour.pyi
"pre-commit.ci autofix"
PR checklist