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

Parsing? #4

Open
matthiasbeyer opened this issue Apr 14, 2023 · 8 comments
Open

Parsing? #4

matthiasbeyer opened this issue Apr 14, 2023 · 8 comments
Labels
feature request New feature or request
Milestone

Comments

@matthiasbeyer
Copy link

matthiasbeyer commented Apr 14, 2023

Hi! Would be really nice if this crate would support parsing as well!

E.G:

assert_eq!("123.5kPackets".parse().unwrap(), HumanCount::new(123456_u32, "Packets"));

Or something like this.

@rsalmei
Copy link
Owner

rsalmei commented Apr 15, 2023

Hey again!

Humm, that's different, but I can do it too...
Only in your example you kinda generated data out of thin air, it would be more like:

assert_eq!("123.5kPackets".parse().unwrap(), HumanCount::new(123500_u32, "Packets"));

Anyway, I'll do this after serde, because this seems to require much more work (tests...).

@matthiasbeyer
Copy link
Author

Hey! Thanks again for considering it! Yes, the example was kinda weird... I don't mind a completely different approach, my main interest here is that I can parse 😄

Depending on how you approach things, it might be worth doing it exactly the other way round, because the serde functionality (#3) would require parsing for its Serialize implementation, right? 😄

But whatever way you prefer! 😆

@rsalmei
Copy link
Owner

rsalmei commented Apr 19, 2023

Hey, please explain exactly why would you need this...
I thought about it a bit more, and it actually doesn't make sense to parse here. The objective of this project is doing the heavy work of converting statistics into beautiful human representations. So, if a program already has such representations printed, why would we parse them? Only to be able to generate the same representations again?

@matthiasbeyer
Copy link
Author

matthiasbeyer commented Apr 19, 2023

Because if it is used in some configuration file, say:

duration = "12ms"

a user might want to parse that. Of course now that the crate implements serde, offering parsing is not more than exposing that basic building block from serde in the API of the crate, I guess.

Exposing the parsing bits from the serde implementation would also offer the possibility for users to implement interactive programs. For example where a user is asked interactively how much a program should wait before retrying, and they can insert "1 minute" in a text-field.

@matthiasbeyer
Copy link
Author

I just looked at how the serde compat was implemented,... it was not implemented as I intended.

What I intended was that I can have a TOML file like this:

duration = "12ms"

And a struct like this:

#[derive(serde::Deserialize)]
struct Config {
    duration: human_repr::HumanDurationData,
}

And parse the former to the latter. What I would need to specify with the current implementation is

[duration]
val = 12.0
unit = "ms"

Which is different than what I had intended.

@rsalmei
Copy link
Owner

rsalmei commented Apr 19, 2023

Well, serde implementation is abstract by design, you or anyone can now include a format like serde_json or serde_toml and you would get exactly that last output.
I understand what you thought but serde is awesome like that, no project need to do any manual parsing at all.

Regarding the duration = 12ms configuration, the place unfortunately is not here, there's no reason human-repr would do that, but I'm sure there should be some crate that does exactly that kind of parsing.

EDIT: I'm implementing it.

@rsalmei
Copy link
Owner

rsalmei commented Apr 19, 2023

On the yesterday's serde release, I put a section about an example of how to use this, look:

use human_repr::{HumanCount, HumanDuration, HumanThroughput};

let now = std::time::Instant::now();
let updated = 3431237; // process something...

println!("Updated {} successfully.", updated.human_count_bytes());
println!("Operation took {}.", now.elapsed().human_duration());
println!("Rate: {}", (updated as f64 / now.elapsed().as_secs_f64()).human_throughput_bytes());

See, human-repr is the helper to make beautiful logs or state messages to inform a user what some program did.
One would always have his statistics ready, like the number of updated items and how many seconds it has elapsed, to make such messages...

I did implement serde, but actually, if one needs to persist or transfer e.g. a HumanCount, one could generate the beautiful data representation it provides and just use that.
I implemented it anyway because I think I could use it in the future to provide some more features, like dividing a HumanCount by a HumanDuration, and giving a HumanThroughput...
For that, it could be an advantage to store them. But parsing, unfortunately, I don't see any reason. It is very different from the purpose of this crate. Yep, I've changed my mind.

@rsalmei
Copy link
Owner

rsalmei commented Apr 25, 2023

Hey @matthiasbeyer, you may have noticed I left this issue open...
I was thinking about it more thoroughly, and it does fit the concept implementing parsing here!
This is human-repr, i.e. it can deal with general human representations! The initial impl had Generation, and now it can provide Parsing...
I'll invest some time in this as soon as I can. I'll also change the serde impl, so it uses this parsing functionality too.

@rsalmei rsalmei added the feature request New feature or request label Jan 29, 2024
@rsalmei rsalmei added this to the 2.0 milestone Aug 28, 2024
@rsalmei rsalmei modified the milestone: 2.0 Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants