Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactoring load module #44

Merged
merged 4 commits into from
Jul 30, 2024
Merged

Refactoring load module #44

merged 4 commits into from
Jul 30, 2024

Conversation

y0z
Copy link
Member

@y0z y0z commented Jul 12, 2024

Motivation

  • Unify the implementation of module import functionality in load_module and load_module_local.

Description of the changes

Refactoring load_module as follows:

  • First, download a target package from GitHub to the local cache directory.
  • Then, call load_local_module to import the target module from the local cache.

Minor refactoring is also applied.

  • Update doctring.
  • To fix the inconsistency, change the order of the ref and registry_root arguments for load_local_module.

@y0z y0z added the code-fix Change that does not change the behavior, such as code refactoring. label Jul 12, 2024
@y0z y0z changed the title Feature/refactoring load module Rrefactoring load module Jul 12, 2024
@y0z y0z changed the title Rrefactoring load module Refactoring load module Jul 19, 2024
@HideakiImamura
Copy link
Member

It would be great to add unit tests to validate the change of this PR does not affect the behaviour. Could you do that?

@y0z
Copy link
Member Author

y0z commented Jul 24, 2024

@HideakiImamura
Thank you for your feedback.
I added tests. PTAL.

HideakiImamura
HideakiImamura approved these changes Jul 30, 2024
View reviewed changes
Copy link
Member

@HideakiImamura HideakiImamura left a comment

Choose a reason for hiding this comment

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

LGTM!

@HideakiImamura HideakiImamura merged commit 15f8df3 into optuna:main Jul 30, 2024
4 checks passed
@HideakiImamura HideakiImamura added this to the v0.1.0 milestone Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HideakiImamura HideakiImamura HideakiImamura approved these changes

Assignees

@HideakiImamura HideakiImamura

Labels
code-fix Change that does not change the behavior, such as code refactoring.
Projects
None yet
Milestone
v0.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@y0z @HideakiImamura