search

EmuKit / emukit

A Python-based toolbox of various methods in decision making, uncertainty quantification and statistical emulation: multi-fidelity, experimental design, Bayesian optimisation, Bayesian quadrature, etc.
https://emukit.github.io/emukit/
Apache License 2.0
605 stars 128 forks source link

added __str__ and __repr__ to emukit/core/*parameter.py #336

Closed ekalosak closed 3 years ago

ekalosak commented 3 years ago

Description of changes: Implemented str and repr magic methods for the Parameter class and XyzParameter subclasses.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

apaleyes commented 3 years ago

Thanks, very useful update!

Two comments:

  1. Can you add new lines between methods, please?
  2. It isn't immediately clear to me wny str and repr display different information. What was your thinking behind that decision?
ekalosak commented 3 years ago
  1. Newlines incoming
  2. The intention behind repr() is to provide developer-friendly output i.e. something a dev can copy and paste to recreate the object being represented. The intention behind str() is to provide user-friendly output i.e. something a user can easily interpret. It's informative to invoke the last line of >>> help(repr): "For many object types, including most builtins, eval(repr(obj)) == obj".

Any thoughts on adding black or some other code formatter to the CI?

apaleyes commented 3 years ago

No, haven't considered any formatters yet. But will now, thanks for asking!

Can you have a look at the build failure? Seems to be some confusion with multi-line f-string.

codecov-io commented 3 years ago

Codecov Report

Merging #336 (d317d9c) into master (184da9f) will decrease coverage by 0.18%. The diff coverage is 62.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #336      +/-   ##
==========================================
- Coverage   89.40%   89.22%   -0.19%     
==========================================
  Files         123      123              
  Lines        4022     4045      +23     
  Branches      459      459              
==========================================
+ Hits         3596     3609      +13     
- Misses        330      340      +10     
  Partials       96       96
Impacted Files Coverage Δ
emukit/core/categorical_parameter.py 95.23% <60.00%> (-4.77%) :arrow_down:
emukit/core/continuous_parameter.py 91.66% <60.00%> (-8.34%) :arrow_down:
emukit/core/discrete_parameter.py 85.00% <60.00%> (-3.89%) :arrow_down:
emukit/core/bandit_parameter.py 64.77% <66.66%> (-0.29%) :arrow_down:
emukit/core/parameter.py 79.16% <66.66%> (-4.17%) :arrow_down:

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 184da9f...d317d9c. Read the comment docs.

ekalosak commented 3 years ago

@apaleyes looks like my hubris in submitting a PR without associated unit tests was misplaced. I've added tests.

apaleyes commented 3 years ago

Yup, tests are always good to have! thanks, merging now