Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display bandwidth in different unit families #328

Merged
merged 7 commits into from
Nov 2, 2023
Merged

Conversation

cyqsimon
Copy link
Collaborator

@cyqsimon cyqsimon commented Nov 1, 2023

  • Added option -u, --unit-family
  -u, --unit-family <UNIT_FAMILY>
          Choose a specific family of units

          [default: bin-bytes]

          Possible values:
          - bin-bytes: bytes, in powers of 2^10
          - bin-bits:  bits, in powers of 2^10
          - si-bytes:  bytes, in powers of 10^3
          - si-bits:   bits, in powers of 10^3
  • Supported families are {binary,SI}-{bytes,bits}

Closes #145; closes #213.

Checklist

  • implementation
  • tests (do we really need them?)

- Supported units are {binary,SI}-{bytes,bits}
This was referenced Nov 1, 2023
@sigmaSd
Copy link
Collaborator

sigmaSd commented Nov 1, 2023

Ok I tried to add some tests via a pr to your branch but I failed miserably , can you add this please, also I have some couple remarks there as well

mod tests {
    use crate::display::BandwidthUnitFamily;

    #[test]
    fn unit_test() {
        // size < 1024 are not calculated correctly
        {
            let size = 1024.;
            assert_eq!(
                BandwidthUnitFamily::BinBytes.get_unit_for(size),
                (1024., "KiB")
            );
            assert_eq!(
                BandwidthUnitFamily::BinBits.get_unit_for(size),
                (128., "Kib")
            );
            // the next 2 `k` are lowercase
            // should  they be uppercase ?
            // they are uppercase on the next units (M G T)
            assert_eq!(
                BandwidthUnitFamily::SiBytes.get_unit_for(size),
                (1000., "kB")
            );
            assert_eq!(BandwidthUnitFamily::SiBits.get_unit_for(size), (125., "kb"));
        }
        {
            let size = 1024. * 1024.;
            assert_eq!(
                BandwidthUnitFamily::BinBytes.get_unit_for(size),
                (1048576.0, "MiB")
            );
            assert_eq!(
                BandwidthUnitFamily::BinBits.get_unit_for(size),
                (131072.0, "Mib")
            );
            assert_eq!(
                BandwidthUnitFamily::SiBytes.get_unit_for(size),
                (1000000.0, "MB")
            );
            assert_eq!(
                BandwidthUnitFamily::SiBits.get_unit_for(size),
                (125000.0, "Mb")
            );
        }
        {
            let size = 1024. * 1024. * 1024.;
            assert_eq!(
                BandwidthUnitFamily::BinBytes.get_unit_for(size),
                (1073741824.0, "GiB")
            );
            assert_eq!(
                BandwidthUnitFamily::BinBits.get_unit_for(size),
                (134217728.0, "Gib")
            );
            assert_eq!(
                BandwidthUnitFamily::SiBytes.get_unit_for(size),
                (1000000000.0, "GB")
            );
            assert_eq!(
                BandwidthUnitFamily::SiBits.get_unit_for(size),
                (125000000.0, "Gb")
            );
        }
        {
            let size = 1024. * 1024. * 1024. * 1024.;
            assert_eq!(
                BandwidthUnitFamily::BinBytes.get_unit_for(size),
                (1099511627776.0, "TiB")
            );
            assert_eq!(
                BandwidthUnitFamily::BinBits.get_unit_for(size),
                (137438953472.0, "Tib")
            );
            assert_eq!(
                BandwidthUnitFamily::SiBytes.get_unit_for(size),
                (1000000000000.0, "TB")
            );
            assert_eq!(
                BandwidthUnitFamily::SiBits.get_unit_for(size),
                (125000000000.0, "Tb")
            );
        }
    }
}

@cyqsimon
Copy link
Collaborator Author

cyqsimon commented Nov 2, 2023

Ah okay. Yeah a unit test will do. I was overcomplicating the situation thinking I need an integration test.

I used some itertools magic to test a wider variety of values. Now I can finally see the appeal of cargo insta 😅.


the next 2 k are lowercase
should they be uppercase ?
they are uppercase on the next units (M G T)

Nope. The k prefix is deliberately lowercase as per ISO 80000.

@cyqsimon
Copy link
Collaborator Author

cyqsimon commented Nov 2, 2023

Also I took a look at the test cases < 1024; I don't really see a problem? Can you please help me verify? Thanks.

@sigmaSd
Copy link
Collaborator

sigmaSd commented Nov 2, 2023

Ah I see, I was misunderstanding what get_unit_for does, so there is no problem indeed. Cool test btw

@cyqsimon
Copy link
Collaborator Author

cyqsimon commented Nov 2, 2023

Okay then. Let's get this merged!

@cyqsimon cyqsimon merged commit 16a6f9e into main Nov 2, 2023
7 of 10 checks passed
@cyqsimon cyqsimon deleted the unit-selection branch November 2, 2023 06:01
@cyqsimon cyqsimon restored the unit-selection branch April 15, 2024 07:02
@cyqsimon cyqsimon deleted the unit-selection branch April 15, 2024 07:09
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.

Feature - bandwidth in Mbit/s Units
2 participants