FEAT: Convert numeric to prefixed value function

[daquintero]

Hi Dan,

Hope you are well! I find this function useful and maybe it could be handy for others.

It works like:

value = 1e4
converted_value = h.prefix.convert_numeric_to_prefix(value)
assert 10 * h.prefix.K == converted_value
value = 1e-4
converted_value = h.prefix.convert_numeric_to_prefix(value)
assert 0.1 * h.prefix.m == converted_value

I've included this in a test too. Just in case it is of any use.

[ThomasPluck]

Hey, thanks for the contribution!

So I'm comparing your function to one that does something pretty similar called hdl21.prefix.to_prefixed, the Prefixed type is designed to be "Prefix-agnostic" - so it just passes the Decimal'd version of whatever float you give it to a Prefixed with a default prefix of UNIT.

However, it doesn't scale the decimal to the most probable choice Prefix, as your feature does here. I do actually remember this bothering me during #79 and for some reason I dropped it - probably out of laziness.

So, two things that I'd recommend before pulling this:

• I'd extend to_prefixed to use the scaling that you propose using the scale method in Prefixed and closest method in Prefix
• Add a __call__ method to Prefixed to override the default behavior of the pydantic BaseModel to allow Prefixed to allow it behave more intuitively. Example below:

from hdl21 import Prefixed
from hdl21.prefix import MILLI

a = Prefixed(0.001234)
assert a == Prefixed(number=Decimal('1.234'),prefix=MILLI)

[daquintero]

Sounds good! I'll send an updated one

[codecov]

Codecov Report

Merging #159 (b37fd4a) into main (137333c) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   87.19%   87.22%   +0.02%     
==========================================
  Files         109      109              
  Lines        9568     9590      +22     
==========================================
+ Hits         8343     8365      +22     
  Misses       1225     1225              

Impacted Files	Coverage Δ
hdl21/prefix.py	96.74% <100.00%> (+0.21%)	⬆️
hdl21/tests/test_prefix.py	100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[dan-fritchman]

Looking great! A few thoughts:

• Agree there's a good amount of overlap with to_prefixed, as well as Prefixed.scale and Prefix.closest
• Maybe make, like, Prefixed.normalize() (or regularize() or something?). Something that takes a Prefixed value, figures out the "preferable" Prefix, and Prefixed.scale()s to that. Idunno what the most typical name for that process would be.
• And maybe make "whether to do that" a (boolean) argument to to_prefixed().
• Or just don't, and you just need two calls, e.g. to_prefixed(1e-6).regularize()
• Definitely don't think we need to restate the list of Prefix values; it's there in the enum definition

Related to, but not part of this PR -

def _scale_to_smaller(
    me: Prefixed, other: Union[Prefixed, ToPrefixed]
) -> Tuple[Prefixed, Prefixed]:
    """# Scale two `Prefixed` numbers to the smaller of the two prefixes.
    The `me` argument is always a `Prefixed`, generally due to being the `self` in a `Prefixed` mthod.
    The `other` argument is commonly another compatible/ convertible type,
    and is converted before scaling."""
    other = to_prefixed(other)
    smaller = me.prefix if me.prefix.value < other.prefix.value else other.prefix
    return me.scale(smaller), other.scale(smaller)

It seems that, without such "normalization"/ "regularization"/ whatever-it's-called... this can be wrong?
E.g. if me is 1_000_000_000 * NANO and other is 1 * MILLI, or similar.

[ThomasPluck]

• Not sure there is a dedicated term for picking the common-sense SI prefix, guess you could go with "autoscale"
• That is a goofy code snippet, I'll roll in a fix with this PR

[daquintero]

All sounds great and cheers for all your work on this really good software. Sorry, I am away of my computer until next week or would help sooner! Sent from Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Thomas Pluck ***@***.***> Sent: Thursday, July 20, 2023 8:31:39 PM To: dan-fritchman/Hdl21 ***@***.***> Cc: Dario Antonio Quintero Dominguez ***@***.***>; Author ***@***.***> Subject: Re: [dan-fritchman/Hdl21] FEAT: Convert numeric to prefixed value function (PR #159) * Not sure there is a dedicated term for picking the common-sense SI prefix, guess you could go with "autoscale" * That is a goofy code snippet, I'll roll in a fix with this PR — Reply to this email directly, view it on GitHub<#159 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIO5PRGQ75TG5DOJGCTHKTLXRGBRXANCNFSM6AAAAAA2M55WBM>. You are receiving this because you authored the thread.Message ID: ***@***.***>