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

Update repr for dataset/workflow classes and add uri kwarg for QlibRecorder #302

Merged
merged 8 commits into from
Mar 8, 2021
Merged

Update repr for dataset/workflow classes and add uri kwarg for QlibRecorder #302

merged 8 commits into from
Mar 8, 2021

Conversation

D-X-Y
Copy link
Contributor

@D-X-Y D-X-Y commented Mar 3, 2021 •
edited
Loading

Update String Representation for DatasetH and ExpManager / Add uri kwarg for QlibRecorder

Description

  • Make the string representation of DatasetH and ExpManager look nicer.
  • Let QlibRecorder can externally set uri.

How Has This Been Tested?

  • Pass the test by running: pytest qlib/tests/test_all_pipeline.py under upper directory of qlib.
  • If you are adding a new feature, test on your own test scripts.

Screenshots of Test Results (if appropriate):

image

image

Types of changes

  • Fix bugs
  • Add new feature
  • Update documentation

@ghost
Copy link

ghost commented Mar 3, 2021 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@you-n-g you-n-g requested a review from Derek-Wds March 3, 2021 07:10
Derek-Wds
Derek-Wds reviewed Mar 4, 2021
View reviewed changes
qlib/workflow/expm.py Show resolved Hide resolved
qlib/workflow/__init__.py Show resolved Hide resolved
@D-X-Y D-X-Y requested a review from Derek-Wds March 4, 2021 05:04
Derek-Wds
Derek-Wds reviewed Mar 4, 2021
View reviewed changes
Copy link
Contributor

@Derek-Wds Derek-Wds left a comment

Choose a reason for hiding this comment

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

Thanks~

Regarding to the unexpected mlruns folder error, please delete this line, as well as line 179 (which is the comment).

The set_experiment method in mlflow is useless under our current version.

@D-X-Y
Copy link
Contributor Author

D-X-Y commented Mar 4, 2021

Thanks! It is fixed now.

Derek-Wds
Derek-Wds approved these changes Mar 4, 2021
View reviewed changes
Copy link
Contributor

@Derek-Wds Derek-Wds left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the help!

@Derek-Wds Derek-Wds added bug Something isn't working enhancement New feature or request labels Mar 4, 2021
@D-X-Y
Copy link
Contributor Author

D-X-Y commented Mar 4, 2021

@you-n-g Would you mind helping merge this PR?

you-n-g
you-n-g reviewed Mar 5, 2021
View reviewed changes
tests/test_all_pipeline.py Outdated Show resolved Hide resolved
qlib/workflow/expm.py Outdated Show resolved Hide resolved
qlib/workflow/recorder.py Outdated Show resolved Hide resolved
qlib/workflow/__init__.py Show resolved Hide resolved
qlib/workflow/__init__.py Outdated Show resolved Hide resolved
@D-X-Y D-X-Y requested review from you-n-g and Derek-Wds March 5, 2021 06:42
@D-X-Y D-X-Y changed the title Update repr for DatasetH and ExpManager / Add uri kwarg for QlibRecorder Update repr for dataset/workflow classes and add uri kwarg for QlibRecorder Mar 5, 2021
Derek-Wds
Derek-Wds approved these changes Mar 8, 2021
View reviewed changes
Copy link
Contributor

@Derek-Wds Derek-Wds left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks~

@D-X-Y
Copy link
Contributor Author

D-X-Y commented Mar 8, 2021

@Derek-Wds Thanks :)

@you-n-g you-n-g merged commit 91eef93 into microsoft:main Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Derek-Wds Derek-Wds Derek-Wds approved these changes

@you-n-g you-n-g Awaiting requested review from you-n-g

Assignees
No one assigned
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@D-X-Y @you-n-g @Derek-Wds