-
Notifications
You must be signed in to change notification settings - Fork 458
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
Minutizer #231
base: master
Are you sure you want to change the base?
Minutizer #231
Conversation
…es from strings like 'It lasted 2 hours and 30 mintues'
…t as a string and return the duration found in that text as an integer/fixnum. ex: Minutizer.new.minutize("It took me a 5 hours to write my tests") => 300 ex2: Minutizer.new.minituze("I spent 321 seconds contemplating this commit message") => 5.35
…izing a new instance of Minutizer
I will also write up an explanation of minutize in the README and add it to the pull request if you want to add this. |
I'm not sure if Chronic is right gem for this kind of functionality. Chronic has always parsed only discrete times and not durations. Durations are parsed with chronic_duration gem |
Also I don't think it's good idea to use minutes, it doesn't matter what you've to display to user, but most people would expect to keep seconds internally and to show only minutes would just |
Yeah, I was thinking about changing it to display seconds (would also require a name change). The one difference between this and ChronicDuration is CD is set to parse duration from specific strings like '4m20sec' where Minutizer parsers duration from text more similar to what Chronic would receive. Something a little more Chronic might be to create a Chronic#parse_span method that incorporates what I've written for Minutizer. |
CD also parses natural texts, for example
Most of these tests does pass for CD and I think it might be very useful to improve CD even more if you need some specific features (eg. Span for CD). Primarily because Chronic just isn't really meant for it and then when there will be need for more duration features we'll have to reimplement lot of stuff that's already in CD. Anyway it's up to @leejarvis Another thing, I think that |
Regarding Chronic.parse_span, I was thinking it would be for sentences like: 'since last tuesday'
'until next tuesday'
'last night until this morning'
'yesterday until tomorrow'
'yesterday at 5pm until tomorrow at 8am'
'8am - 12pm'
'the last 3 days'
'the next 30 minutes'
'since August 30th' which come from test code I've already written for span creation. parsing spans for these kind outside of Chronic would require recreating/copying a lot of what Chronic currently does. |
that would be useful functionality and would fit fine for Chronic, but I don't see how that's related to this Minutizer. When you parse such duration, it would return Span and with that can easily get length in seconds. Only currently Chronic doesn't really have any real support for such parsing, but for that feature 👍 if you implement. |
For most of the spans of time, like "yesterday until tomorrow," you don't need a duration. But for "next 3 hours" or "past 30 minutes" I believe you need some kind of duration or representation of that time difference from now. Minutizer was built for the end goal of getting spans using Chronic, which is why I sent a pull request for it first. if @leejarvis gives parse_span the go ahead, I can build that feature for Chronic. |
It does make sense. And if "next 3 hours" returns span then yes it's fine for Chronic. Also Minutizer isn't really needed, because Chronic already have somewhat duration support internally, for example Chronic parses "after 3 hours" fine. So what I mean, Chronic shouldn't expose such functionality outside and use durations only internally which it already does (take a look at repeaters). Basically such span processing should happen same as typical parse - first we split input text in tokens, then we process each token and do rest stuff, etc. So we don't need Minutizer as we already have tokens ready for processing and that's why I'm saying Minutizer isn't good fit here but rather CD. Also Chronic support for durations is very poor as it's not DST aware (here's several issues opened about it) and should be reimplemented completely. For next week it uses week_start + WEEK_SECONDS and your Minutizer doesn't do any better. Besides seems |
I like this, but I agree that it could be a confusing thing to add to Chronic. I also agree something like this should always return seconds over minutes. I'm happy to have a |
Add Chronic.minutize(text) to extract the duration from text.
I use this personally to extract the duration of events from updates or events to set the duration, start and end times, etc. I chose minutes, because most information displayed to users is in minutes or hours, but not seconds.
Some examples of use:
Another cool thing you can do with this new feature is utilize Chronic's Span to return a span based on the text's create_at (or another anchoring time).
This is also useful to use during a model's callbacks, such as before_create or before_save.