search

kubeflow / katib

Automated Machine Learning on Kubernetes
https://www.kubeflow.org/docs/components/katib
Apache License 2.0
1.51k stars 441 forks source link

[enhancement] Should not use Float in CRD definition. #889

Open gaocegege opened 5 years ago

gaocegege commented 5 years ago

/kind feature

Describe the solution you'd like [A clear and concise description of what you want to happen.]

Ref https://github.com/kubernetes-sigs/controller-tools/issues/245

so, it's technically supported, but floats are highly discouraged (and disallowed in official kubernetes APIs) because they won't round-trip properly, can't actually represent certain numbers exactly, etc (all the normal reasons floats are usually not what you want in a program). We recommend using resource.Quantity or int32 for your numeric types, in accordance with the Kubernetes API conventions.

Anything else you would like to add: [Miscellaneous information that will assist in solving the issue.]

gaocegege commented 4 years ago

/priority p3

gaocegege commented 4 years ago

We can use https://golang.org/src/encoding/json/decode.go#L187 json.Number.

issue-label-bot[bot] commented 4 years ago

Issue Label Bot is not confident enough to auto-label this issue. See dashboard for more details.

issue-label-bot[bot] commented 4 years ago

Issue-Label Bot is automatically applying the labels:

Label Probability
area/front-end 0.83

Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback! Links: app homepage, dashboard and code for this bot.

andreyvelich commented 4 years ago

@gaocegege Is it better than just a simple string, like we did with metrics values ?

gaocegege commented 4 years ago

json.Number is a wrapper of string. We can use string, too.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

andreyvelich commented 3 years ago

/lifecycle frozen