Skip to content

Conversation

@jdrew82
Copy link
Contributor

@jdrew82 jdrew82 commented Feb 9, 2022

This PR addresses issue #85. There appears to be a quirk with the round method that it will show the number as a float if you specify 0 decimal places and as an int, if you don't specify the places at all. This tweak should address the quirk and represent the name as expected.

There appears to be a quirk with the round method that it will show the number as a float if you specify 0 decimal places and as an int if you don't specify the places at all. This tweak should fix the issue.
Copy link
Collaborator

@jeffkala jeffkala left a comment

Choose a reason for hiding this comment

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

Looks good.

if val["low"] <= speed < val["high"]:
try:
return f"{round(speed / val['low'], nbr_decimal)}{bit_type}"
if nbr_decimal != 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

To keep it a bit more dry, does this make sense?

if nbr_decimal == 0
    nbr_decimal = None
>>> round(1500 / 100, None)
15
>>> round(1500 / 100, 0)
15.0
>>>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I've refactored this bit to remove duplication and simplify it.

{"sent": {"speed": 1000000000, "nbr_decimal": 1}, "received": "1.0Gbps"},
{"sent": {"speed": 1000000000000}, "received": "1.0Tbps"},
{"sent": {"speed": 1000000000000}, "received": "1Tbps"},
{"sent": {"speed": 1000000000000, "nbr_decimal": 1}, "received": "1.0Tbps"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add one example of a non-zero?

jdrew82 and others added 4 commits March 3, 2022 16:09
There appears to be a quirk with the round method that it will show the number as a float if you specify 0 decimal places and as an int if you don't specify the places at all. This tweak should fix the issue.

refactor: ♻️ Simplify code to reduce code duplication
@itdependsnetworks itdependsnetworks merged commit fcda3af into networktocode:develop Mar 4, 2022
@jdrew82 jdrew82 deleted the issue_85 branch March 15, 2022 19:43
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.

3 participants