Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
ksunden merged 1 commit intomatplotlib:mainfromDHClimber:depreciated
Nov 16, 2023
Merged

Removal of deprecations for Contour#26907

ksunden merged 1 commit intomatplotlib:mainfromDHClimber:depreciated
Nov 16, 2023

Conversation

DHClimber
Copy link
Contributor

@DHClimberDHClimber commentedSep 24, 2023
edited
Loading

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

Copy link

@github-actionsgithub-actionsbot left a 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.

Copy link
Member

@ksundenksunden left a 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:

``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.

DHClimber reacted with thumbs up emoji
@ksundenksunden changed the titledeprecatedRemoval of deprecations for ContourSep 25, 2023
@ksundenksunden mentioned this pull requestSep 25, 2023
14 tasks
Copy link
Member

@ksundenksunden left a 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

Comment on lines 1 to 5
Removal change template
~~~~~~~~~~~~~~~~~~~~~~~

Enter description of methods/classes removed here....

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Removal change template
~~~~~~~~~~~~~~~~~~~~~~~
Enter description of methods/classes removed here....

Comment on lines 18 to 19
Please rename file with PR number and your initials i.e. "99999-ABC.rst"
and ``git add`` the new file.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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

@DHClimber
Copy link
ContributorAuthor

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

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.

@ksunden
Copy link
Member

We have some docs on squashing viagit rebase -i:

https://matplotlib.org/stable/devel/development_workflow.html#rewrite-commit-history

@ksunden
Copy link
Member

Also remove from the corresponding.pyi file, please

@DHClimber
Copy link
ContributorAuthor

We have some docs on squashing viagit rebase -i:

https://matplotlib.org/stable/devel/development_workflow.html#rewrite-commit-history

Thanks, I think I figured it out.

@DHClimber
Copy link
ContributorAuthor

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?

@rcomer
Copy link
Member

I agree the Mypy message could be clearer, but Ithink it’s just that you have removed some methods from the.py file that you have not yet removed from the.pyi file.

Copy link
Member

@ksundenksunden left a 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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
attribute and the font properties of the corresponding text objects instead.
attribute and the font properties of the corresponding text objects instead.

@@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""%_docstring.interpd.params)
"""%_docstring.interpd.params)

ksunden
ksunden previously approved these changesSep 27, 2023
@QuLogic
Copy link
Member

QuLogic commentedSep 27, 2023
edited
Loading

stubtest is still complaining; the entries corresponding to the removed ones need to be removed from the.pyi file as well. You can see its comments on theFiles changed tab.

@DHClimber
Copy link
ContributorAuthor

I'm completely lost at this point. I don't understand where all the problems are coming from.

@ksunden
Copy link
Member

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 theinheritance-diagram that you had to update, which is causing a conflict.

Unfortunately that is one big long line that makes it hard to tell what the differences are:

It was edited in#26874, which removed theBrokenBarHCollection, so both that andCLabelText should be removed.

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.

@DHClimber
Copy link
ContributorAuthor

e is that main has also updated the

Thanks, I'll give it one more try. Thanks for the patience.

@QuLogic
Copy link
Member

QuLogic commentedOct 3, 2023
edited
Loading

@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 theupstream remote as we recommend):

$ git fetch upstream f448c0fc73a624edf46806db961392518b365358:original-depreciated

Then you can reset your branch to it:

$ git checkout depreciated$ git reset --hard original-depreciated

and push to replace it here:

$ git push --force-with-lease origin depreciated

@DHClimber
Copy link
ContributorAuthor

@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 theupstream remote as we recommend):

$ git fetch upstream f448c0fc73a624edf46806db961392518b365358:original-depreciated

Then you can reset your branch to it:

$ git checkout depreciated$ git reset --hard original-depreciated

and push to replace it here:

$ git push --force-with-lease origin depreciated

Thank you, I have been out of town traveling. I'll try to fix now.

@DHClimber
Copy link
ContributorAuthor

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.

@melissawm
Copy link
Member

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
Copy link
ContributorAuthor

DHClimber commentedOct 10, 2023
edited
Loading

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.

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?

@ksunden
Copy link
Member

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

DHClimber reacted with hooray emoji

@DHClimber
Copy link
ContributorAuthor

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.

@QuLogic
Copy link
Member

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.

@DHClimber
Copy link
ContributorAuthor

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.

Sorry about that, should be corrected now.

@QuLogicQuLogic added this to thev3.9.0 milestoneOct 11, 2023
@@ -16,7 +16,7 @@ from collections.abc import Callable, Iterable, Sequence
from typing import Literal
from .typing import ColorType

class ClabelText(Text): ...

Copy link
Member

Choose a reason for hiding this comment

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

Remove this line instead.

Suggested change

@DHClimber
Copy link
ContributorAuthor

Just following up, is there anything else that needs to be done here?

@QuLogic
Copy link
Member

We require two approvals before merges, so this is waiting for a second reviewer.

@ksundenksunden merged commitb16031c intomatplotlib:mainNov 16, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

@QuLogicQuLogicQuLogic approved these changes

@ksundenksundenksunden approved these changes

Assignees
No one assigned
Projects
Milestone
v3.9.0
Development

Successfully merging this pull request may close these issues.

5 participants
@DHClimber@ksunden@rcomer@QuLogic@melissawm

[8]ページ先頭

©2009-2025 Movatter.jp