Skip to content

Adapt TYPE_CHECKING of samplers/_gp/sampler.py#6059

Merged
nabenabe0928 merged 1 commit intooptuna:masterfrom
nabenabe0928:code-fix/fix-type-checking-of-gp-sampler-py
May 7, 2025
Merged

Adapt TYPE_CHECKING of samplers/_gp/sampler.py#6059
nabenabe0928 merged 1 commit intooptuna:masterfrom
nabenabe0928:code-fix/fix-type-checking-of-gp-sampler-py

Conversation

@nabenabe0928
Copy link
Copy Markdown
Contributor

Motivation

This PR relates to:

Description of the changes

  • Fix the TYPE_CHECKING as mentioned in the PR title

@nabenabe0928 nabenabe0928 marked this pull request as ready for review April 23, 2025 00:24
@HideakiImamura
Copy link
Copy Markdown
Member

@gen740 @kAIto47802 Could you review this PR?

@nabenabe0928 nabenabe0928 assigned y0z and unassigned gen740 Apr 28, 2025
@nabenabe0928
Copy link
Copy Markdown
Contributor Author

@y0z Could you review this PR?

Copy link
Copy Markdown
Collaborator

@kAIto47802 kAIto47802 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 the PR!

As for the type annotations, the return type -> None is not needed if at least one argument is annotated, since then the function is considered as "typed."1
However, this is orthogonal to the motivation behind the issue:

Therefore, it's also acceptable to leave this change out of the current PR.

Footnotes

  1. https://mypy.readthedocs.io/en/stable/class_basics.html#annotating-init-methods

@nabenabe0928
Copy link
Copy Markdown
Contributor Author

@kAIto47802
Thank you for the comment!
In fact, -> None in the __init__ constructor is intended, please refer to PEP:

But your remark is also important:)

@y0z
Copy link
Copy Markdown
Member

y0z commented May 7, 2025

LGTM

@y0z y0z removed their assignment May 7, 2025
@y0z y0z added the enhancement Change that does not break compatibility and not affect public interfaces, but improves performance. label May 7, 2025
@kAIto47802 kAIto47802 removed their assignment May 7, 2025
@nabenabe0928 nabenabe0928 merged commit 022f97d into optuna:master May 7, 2025
15 checks passed
@nabenabe0928 nabenabe0928 added this to the v4.4.0 milestone May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Change that does not break compatibility and not affect public interfaces, but improves performance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants