Skip to content

add check for pcs_new read#203

Merged
mfeurer merged 3 commits intoautoml:masterfrom
dengdifan:pcs_read
Dec 14, 2021
Merged

add check for pcs_new read#203
mfeurer merged 3 commits intoautoml:masterfrom
dengdifan:pcs_read

Conversation

@dengdifan
Copy link
Contributor

convert the corresponding forbidden values correctly to the target hyperparameters type

@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #203 (916be28) into master (5f1e3d3) will increase coverage by 0.82%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #203      +/-   ##
==========================================
+ Coverage   66.33%   67.15%   +0.82%     
==========================================
  Files          17       17              
  Lines        1613     1629      +16     
==========================================
+ Hits         1070     1094      +24     
+ Misses        543      535       -8     
Impacted Files Coverage Δ
ConfigSpace/read_and_write/pcs_new.py 90.90% <75.00%> (+2.90%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f1e3d3...916be28. Read the comment docs.

Copy link
Contributor

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I left a few comments. There's currently one PEP8 issue that would also require your attention. You can check locally by doing pip install pre-commit to install pre-commit, pre-commit installto install all packages required to run the checks (flake8 etc) and finallypre-commit run` to do the checks.

@dengdifan dengdifan requested a review from mfeurer December 13, 2021 15:29
Copy link
Contributor

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

This looks good, thank you. Do you think such casting needs to be applied for the condition statements as well?

@mfeurer mfeurer merged commit b6aa830 into automl:master Dec 14, 2021
github-actions bot pushed a commit that referenced this pull request Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants