Skip to content

Update _upload.py for TYPE_CHECKING#6327

Merged
not522 merged 6 commits intooptuna:masterfrom
satyarth7srivastava:type_check
Nov 10, 2025
Merged

Update _upload.py for TYPE_CHECKING#6327
not522 merged 6 commits intooptuna:masterfrom
satyarth7srivastava:type_check

Conversation

@satyarth7srivastava
Copy link
Copy Markdown
Contributor

@satyarth7srivastava satyarth7srivastava commented Nov 4, 2025 •
edited
Loading

Motivation

Updated the _upload.py

Description of the changes

this PR relates #6029

nabenabe0928
nabenabe0928 reviewed Nov 5, 2025
View reviewed changes
nabenabe0928
nabenabe0928 reviewed Nov 5, 2025
View reviewed changes
satyarth7srivastava and others added 2 commits November 5, 2025 12:12
@satyarth7srivastava @nabenabe0928
Update optuna/artifacts/_upload.py
18bbe0b 
Co-authored-by: Shuhei Watanabe <47781922+nabenabe0928@users.noreply.github.com>
@satyarth7srivastava @nabenabe0928
Update optuna/artifacts/_upload.py
d181c4e 
Co-authored-by: Shuhei Watanabe <47781922+nabenabe0928@users.noreply.github.com>
kAIto47802
kAIto47802 approved these changes Nov 5, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@kAIto47802 kAIto47802 left a comment •
edited
Loading

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. It's almost LGTM
Could you please update the PR comment from “resolves” to “relates” (or something similar), since otherwise the original issue will be automatically closed when this PR is merged?

kAIto47802
kAIto47802 requested changes Nov 5, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@kAIto47802 kAIto47802 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Also, could you please apply the formatter to reorder the imports and fix the CI issue?
You can do this by running ./formats.sh at the root of the Optuna directory.

@satyarth7srivastava
Copy link
Copy Markdown
Contributor Author

satyarth7srivastava commented Nov 6, 2025

Also, could you please apply the formatter to reorder the imports and fix the CI issue? You can do this by running ./formats.sh at the root of the Optuna directory.

I'm on it

@not522 not522 self-assigned this Nov 6, 2025
@satyarth7srivastava
Copy link
Copy Markdown
Contributor Author

satyarth7srivastava commented Nov 6, 2025

Updated the file and sorry for inconvenience, I'm new to contributing still learning

kAIto47802
kAIto47802 approved these changes Nov 7, 2025
View reviewed changes
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 update. LGTM

not522
not522 approved these changes Nov 10, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@not522 not522 left a comment

Choose a reason for hiding this comment

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

LGTM!

@not522 not522 added the code-fix Change that does not change the behavior, such as code refactoring. label Nov 10, 2025
@not522 not522 added this to the v4.7.0 milestone Nov 10, 2025
@not522 not522 changed the title updated _upload.py for TYPE_CHECKING Update _upload.py for TYPE_CHECKING Nov 10, 2025
@not522 not522 merged commit 5506e28 into optuna:master Nov 10, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@not522 not522 not522 approved these changes

@kAIto47802 kAIto47802 kAIto47802 approved these changes

+1 more reviewer

@nabenabe0928 nabenabe0928 nabenabe0928 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@not522 not522

Labels

code-fix Change that does not change the behavior, such as code refactoring.

Projects

None yet

Milestone

v4.7.0

Development

Successfully merging this pull request may close these issues.

4 participants

@satyarth7srivastava @not522 @nabenabe0928 @kAIto47802