Skip to content

MNT: Make get_default_filetype a classmethod#31867

Merged
ksunden merged 1 commit into
matplotlib:mainfrom
timhoffm:mnt-get_default_filetype
Jun 10, 2026
Merged

MNT: Make get_default_filetype a classmethod#31867
ksunden merged 1 commit into
matplotlib:mainfrom
timhoffm:mnt-get_default_filetype

Conversation

@timhoffm

@timhoffm timhoffm commented Jun 9, 2026

Copy link
Copy Markdown
Member

FigureCanvasBase defines get_default_filetype as a class method, but all derived implementations overwrite this with an instance method. All implementations could even be static. But let's coerce to a classmethod for consistency and to not change the public API of FigureCanvasBase

@classmethod
def get_default_filetype(cls):

FigureCanvasBase defines get_default_filetype as a classmethod,
but all derived implementations overwrite this with an instance
method. All implementations could even be static. But let's
coerce to a classmethod for consistency and to not change the
public API of FigureCanvasBase
@ksunden ksunden merged commit 0677cc8 into matplotlib:main Jun 10, 2026
41 checks passed
@QuLogic

QuLogic commented Jun 10, 2026

Copy link
Copy Markdown
Member

Oh, I just realized #31867 was merged already; should we not remove the ignore from the type hints as well?

@ksunden

ksunden commented Jun 10, 2026

Copy link
Copy Markdown
Member

I think you were meaning to reference something else, that is the number for this...

@timhoffm timhoffm deleted the mnt-get_default_filetype branch June 10, 2026 21:43
@timhoffm

timhoffm commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

You likely mean #31854.

Yes,

# FIXME: `get_default_filetype` does not inherit from `FigureCanvasBase` correctly
def get_default_filetype(cls) -> str: ... # type: ignore[override]
should be adapted a follow-up.

@myzhang1029

Copy link
Copy Markdown
Contributor

You likely mean #31854.

Yes,

# FIXME: `get_default_filetype` does not inherit from `FigureCanvasBase` correctly
def get_default_filetype(cls) -> str: ... # type: ignore[override]

should be adapted a follow-up.

I was just checking back at this PR. May I go ahead and do this? mypy seems happy now.

timhoffm pushed a commit that referenced this pull request Jun 12, 2026
timhoffm added a commit that referenced this pull request Jun 12, 2026
MNT: add `@classmethod` and remove `ignore` post #31867
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants