add decimal_si_to_f

[zeari]

Needed in order to handle https://github.com/kubernetes/kubernetes/blob/4def5add114b651c26fe576b7315f8029bfce46a/vendor/github.com/appc/spec/schema/types/resource/quantity.go#L49 @cben @moolitayer @zakiva Please review cc @simon3z

[simon3z]

@cben @zeari remember that Quantity is a fixed point variable, it would be nice to try and keep it fixed (vs current floating).

[moolitayer]

@cben @zeari remember that Quantity is a fixed point variable, it would be nice to try and keep it fixed (vs current floating).

In that case we should also check the column type this is eventually stored into

[simon3z]

In that case we should also check the column type this is eventually stored into

@moolitayer what do you mean by "check the column type"? when you do a to_f or a to_i how do you check the column type that you'll use for those?

[moolitayer]

@moolitayer what do you mean by "check the column type"? when you do a to_f or a to_i how do you check the column type that you'll use for those?

The numeric types of the columns these are stored into in Postrgres need not be real or double precision AFAIK

[simon3z]

The numeric types of the columns these are stored into in Postrgres need not be real or double precision AFAIK

@moolitayer yes, that's correct but it's out of scope here (generic lib).

[zeari]

@moolitayer @simon3z IIUC that means i would have to convert it to the smallest possible unit which is "m" = mili. is that what we want here? because that would be weird for bytes

[zeari]

we can also go with something like string.decimal_si_to_f(desired_unit) and desired unit would be 1 of "m", "k", "M".... "none" and itll convert it to that unit

[Fryguy]

we can also go with something like string.decimal_si_to_f(desired_unit) and desired unit would be 1 of "m", "k", "M".... "none" and itll convert it to that unit

ActiveSupport doesn't have these reverse converters, instead choosing to implement them at the presentation layer via the NumberHelper view helper. We implemented more of these inManageIQ's NumberHelper as well.

I'm not sure what is the "right" approach for these reverse converters.

EDIT: It seems ActiveSupport patches to_s in order to expose the NumberHelper methods. https://api.rubyonrails.org/classes/ActiveSupport/NumericWithFormat.html This is really interesting and probably something we might want to extend for ManageIQ's additional number helpers

[simon3z]

I saw good comments about style and generalization (that should be addressed). Anyway ATM overall :+1: for the case we need to cover. So feel free to merge when ready.

[simon3z]

@moolitayer @simon3z IIUC that means i would have to convert it to the smallest possible unit which is "m" = mili. is that what we want here? because that would be weird for bytes

@zeari yes for our use-case I would have preferred to go that way (use milli as unit). But that seems very specific to our use-case.

Anyway either we use something very specific (in the kubernetes parser) or we go with something more generic as this. I understand the pros/cons.

I don't think we should hold this PR on the above discussion (that maybe we can continue somewhere else).

[zeari]

Comments addressed. This also handles decimal exponents.

tests included

[zeari]

https://github.com/ManageIQ/manageiq-providers-kubernetes/pull/22 is also coordinated with this

[moolitayer]

BTW method can be written as

float(to_i) *
{
  "m" => 1e-3, "k" => 1e3, "M" => 1e6, "G" => 1e9, "T" => 1e12, "P" => 1e15, "E" => 1e18, nil => 1
}[
  self[-1]
]

but that might be more confusing...

[zeari]

With this extension and iec60027_2, it feels like it would be nicer to have one method that handled all of the cases. Maybe something like string_to_numeric. What do you think @zeari ?

Sure, lets get this in and do that in separate PR though.

[miq-bot]

Checked commit https://github.com/zeari/more_core_extensions/commit/fd28b8bf0e311633e55b4eeb941ea5504bb13b0a with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 3 files checked, 0 offenses detected Everything looks fine. :star:

[zeari]

@bdunne all comments addressed

[zeari]

ping @bdunne @Fryguy

[zeari]

@bdunne When can this be available on master?

[Fryguy]

@zeari We are just getting in a few more extractions from ManageIQ, and then we plan to cut a release. I'm hoping in the next couple of days.

[bdunne]

@zeari Version 3.3.0 has been released with these changes

[zeari]

Thanks @bdunne @Fryguy