* [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string
@ 2021-07-09 6:13 nobu
2021-07-09 6:30 ` [ruby-core:104554] " samuel
` (24 more replies)
0 siblings, 25 replies; 26+ messages in thread
From: nobu @ 2021-07-09 6:13 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been reported by nobu (Nobuyoshi Nakada).
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:104554] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
@ 2021-07-09 6:30 ` samuel
2021-07-09 7:51 ` [ruby-core:104559] " nobu
` (23 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: samuel @ 2021-07-09 6:30 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by ioquatix (Samuel Williams).
It looks like good improvement.
But does any other `#new` implementation work this way? Can we fix `Time#parse`? I wonder if user will be confused, should they use `Time.parse` or `Time.new`?
Why is `Time.parse` so slow?
This seems like a good performance improvement. But rather than fixing the existing `#parse`, we introduce new one. Now we move the complexity to the user.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-92833
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:104559] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
2021-07-09 6:30 ` [ruby-core:104554] " samuel
@ 2021-07-09 7:51 ` nobu
2021-07-09 8:29 ` [ruby-core:104562] " samuel
` (22 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu @ 2021-07-09 7:51 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
ioquatix (Samuel Williams) wrote in #note-2:
> But does any other `#new` implementation work this way? Can we fix `Time#parse`? I wonder if user will be confused, should they use `Time.parse` or `Time.new`?
The reason not to "fix `Time#parse`" is
> `Time.parse` often results in unintentional/surprising results.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-92840
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:104562] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
2021-07-09 6:30 ` [ruby-core:104554] " samuel
2021-07-09 7:51 ` [ruby-core:104559] " nobu
@ 2021-07-09 8:29 ` samuel
2021-07-09 8:51 ` [ruby-core:104564] " nobu
` (21 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: samuel @ 2021-07-09 8:29 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by ioquatix (Samuel Williams).
> Time.parse often results in unintentional/surprising results.
Can we change it so that it doesn't return unintentional/surprising results?
I agree clean room implementation is nice. But it `Time.parse` can do unexpected things, a new interface does not fix existing usage so the problem still exists.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-92842
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:104564] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (2 preceding siblings ...)
2021-07-09 8:29 ` [ruby-core:104562] " samuel
@ 2021-07-09 8:51 ` nobu
2021-07-10 8:42 ` [ruby-core:104575] " jean.boussier
` (20 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu @ 2021-07-09 8:51 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
ioquatix (Samuel Williams) wrote in #note-4:
> > Time.parse often results in unintentional/surprising results.
>
> Can we change it so that it doesn't return unintentional/surprising results? I realise this might be impossible without extending the interface... e.g. `Time.parse(..., surprising: false)` :p
Yes, it seems by design, and changing the behavior will just break something.
I thought that the same name but different behavior method would be more confusing.
> I agree clean room implementation is nice. But if `Time.parse` can do unexpected things, a new interface does not fix existing usage so the problem still exists, and now we have two interfaces for doing largely the same thing - one that "works" and one that does "unintentional/surprising" things.
A wrapper version based on the new `Time.new` would be possible, I think.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-92844
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:104575] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (3 preceding siblings ...)
2021-07-09 8:51 ` [ruby-core:104564] " nobu
@ 2021-07-10 8:42 ` jean.boussier
2021-07-16 2:48 ` [ruby-core:104626] " samuel
` (19 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: jean.boussier @ 2021-07-10 8:42 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by byroot (Jean Boussier).
```
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
This new interface being faster than `Time.iso8601` at parsing `iso8601` seems weird to me. When I have IS0-8601 dates (pretty much the only format I really use), I'd expect that by using the dedicated method to parse them, I get the best possible performance.
Can we optimize `Time.iso8601` in the same way? Otherwise I can already see people moving to the new API and losing strictness by doing so.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-92857
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:104626] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (4 preceding siblings ...)
2021-07-10 8:42 ` [ruby-core:104575] " jean.boussier
@ 2021-07-16 2:48 ` samuel
2021-10-24 23:14 ` [ruby-core:105763] " ioquatix (Samuel Williams)
` (18 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: samuel @ 2021-07-16 2:48 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by ioquatix (Samuel Williams).
> Yes, it seems by design, and changing the behavior will just break something.
I think there is value in the following interface:
`Time.parse(..., strict: true)` where we have some very clearly documented interpretation of `strict`.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-92921
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:105763] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (5 preceding siblings ...)
2021-07-16 2:48 ` [ruby-core:104626] " samuel
@ 2021-10-24 23:14 ` ioquatix (Samuel Williams)
2021-11-10 10:44 ` [ruby-core:106002] " Eregon (Benoit Daloze)
` (17 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-10-24 23:14 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by ioquatix (Samuel Williams).
I checked the latest update.
I'm still not sure about `Time.new(string)` parsing a string.
What about `Time.parse_strict` or `Time.parse_iso8601` etc?
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-94277
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106002] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (6 preceding siblings ...)
2021-10-24 23:14 ` [ruby-core:105763] " ioquatix (Samuel Williams)
@ 2021-11-10 10:44 ` Eregon (Benoit Daloze)
2021-11-10 11:26 ` [ruby-core:106007] " byroot (Jean Boussier)
` (16 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-11-10 10:44 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by Eregon (Benoit Daloze).
Like @ioquatix and @byroot I think `Time.new` taking a iso-like String is not a good idea.
Also it's technically incompatible (the first argument is always the year for `Time.new` and it even accepts strings):
```
> Time.new "1234-02-03T00:00:00+00:34"
=> 1234-01-01 00:00:00 +0034
```
I understand `Time.iso8601 and Time.parse need an extension library, date.` is annoying and not intuitive.
How about simply making `Time.iso8601` a core method?
> Time.iso8601 can't parse Time#inspect string.
One shouldn't parse `inspect` so this should be no problem in practice.
We could add `Time#iso8601` though (in core), I think that's much better than `time.to_datetime.iso8601`.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-94550
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106007] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (7 preceding siblings ...)
2021-11-10 10:44 ` [ruby-core:106002] " Eregon (Benoit Daloze)
@ 2021-11-10 11:26 ` byroot (Jean Boussier)
2021-11-12 11:33 ` [ruby-core:106036] " timcraft (Tim Craft)
` (15 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: byroot (Jean Boussier) @ 2021-11-10 11:26 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by byroot (Jean Boussier).
If I might add a nitpick, the actual format is RFC 3339, which is pretty much a subset of ISO 8601.
But yes, +1 to making `Time.iso8601` a core method.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-94556
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106036] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (8 preceding siblings ...)
2021-11-10 11:26 ` [ruby-core:106007] " byroot (Jean Boussier)
@ 2021-11-12 11:33 ` timcraft (Tim Craft)
2021-12-03 4:07 ` [ruby-core:106440] " nobu (Nobuyoshi Nakada)
` (14 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: timcraft (Tim Craft) @ 2021-11-12 11:33 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by timcraft (Tim Craft).
Nice performance improvement!
In terms of the interface I think it would be confusing to make `Time.new` parse a string:
* Using a `.parse` method for parsing is more conventional / less surprising / more intention revealing
* Having *both* `.new` and `.parse` would be confusing because it's not clear when to use one or the other, what the trade-offs are etc
It feels very unintiutive and not "least surprise". Similarly `Time.parse(...,format: :iso8601)` and `Time.parse_iso8601` feel a bit clunky.
Is this awkwardness stemming from trying to fit much functionality into the Time class? It would be more work, but if there was a dedicated ISO8601 module we could shift some responsibility away from the Time class. For example:
```
ISO8601::Time.parse # strictly ISO8601 parsing, returns a Time object
```
This could be extended to encapsulate ISO8601 parsing for other classes:
```
ISO8601::Date.parse
ISO8601::Duration.parse
ISO8601::TimeInterval.parse
```
The same general pattern could be followed for other formats:
```
RFC3339::Time.parse
RFC2822::Time.parse
SQL92::Time.parse
```
Each of those methods would have the same signature, so they could be easily interchanged to get varying degrees of strictness or performance.
From a naming and readability perspective the names are less surprising, more intention revealing, and very greppable.
The higher level `Time.parse` method could potentially delegate to these methods to support parsing a wider range of formats.
Each module could also be extended to encapsulate string formatting as an alternative to adding instance methods like #iso8601, #rfc3339 etc.
***
An alternative to using the formats as the top level modules would be to invert and nest the modules under the Time classes, for example:
```
Time::ISO8601.parse # instead of ISO8601::Time.parse
```
Same benefits—I'm not sure which would be considered more Ruby-ish?
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-94627
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106440] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (9 preceding siblings ...)
2021-11-12 11:33 ` [ruby-core:106036] " timcraft (Tim Craft)
@ 2021-12-03 4:07 ` nobu (Nobuyoshi Nakada)
2021-12-03 6:54 ` [ruby-core:106443] " nobu (Nobuyoshi Nakada)
` (13 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-12-03 4:07 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
Eregon (Benoit Daloze) wrote in #note-10:
> Also it's technically incompatible (the first argument is always the year for `Time.new` and it even accepts strings):
> ```
> > Time.new "2234-01-01T00:00:00+07:00"
> => 2234-01-01 00:00:00 +0100
> ```
I overlooked it.
May #18331 be better?
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95085
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106443] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (10 preceding siblings ...)
2021-12-03 4:07 ` [ruby-core:106440] " nobu (Nobuyoshi Nakada)
@ 2021-12-03 6:54 ` nobu (Nobuyoshi Nakada)
2021-12-03 10:08 ` [ruby-core:106449] " sawa (Tsuyoshi Sawada)
` (12 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-12-03 6:54 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
I remember #18331 won't work with subclasses of `Time`.
Does `Time.at("2021-12-04 02:00:00")` make sense?
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95091
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106449] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (11 preceding siblings ...)
2021-12-03 6:54 ` [ruby-core:106443] " nobu (Nobuyoshi Nakada)
@ 2021-12-03 10:08 ` sawa (Tsuyoshi Sawada)
2021-12-03 10:23 ` [ruby-core:106451] " sawa (Tsuyoshi Sawada)
` (11 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: sawa (Tsuyoshi Sawada) @ 2021-12-03 10:08 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by sawa (Tsuyoshi Sawada).
nobu (Nobuyoshi Nakada) wrote in #note-15:
> I remember #18331 won't work with subclasses of `Time`.
Can you elaborate on that? What problem does it have? Even more helpful if you comment on #18331.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95104
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106451] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (12 preceding siblings ...)
2021-12-03 10:08 ` [ruby-core:106449] " sawa (Tsuyoshi Sawada)
@ 2021-12-03 10:23 ` sawa (Tsuyoshi Sawada)
2021-12-03 11:33 ` [ruby-core:106452] " Eregon (Benoit Daloze)
` (10 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: sawa (Tsuyoshi Sawada) @ 2021-12-03 10:23 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by sawa (Tsuyoshi Sawada).
nobu (Nobuyoshi Nakada) wrote in #note-15:
> I remember #18331 won't work with subclasses of `Time`.
Can you elaborate on that? Is your concern about certain frameworks using time-like classes other than `Time`? I don't see any problem with that. If you create a `Time` object using `Kernel.#Time` proposed in #18331, that can easily be converted (often without noticing) to such class in case of Ruby on Rails' `ActiveSupport::TimeWithZone`.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95106
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106452] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (13 preceding siblings ...)
2021-12-03 10:23 ` [ruby-core:106451] " sawa (Tsuyoshi Sawada)
@ 2021-12-03 11:33 ` Eregon (Benoit Daloze)
2021-12-03 13:21 ` [ruby-core:106456] " nobu (Nobuyoshi Nakada)
` (9 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-12-03 11:33 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by Eregon (Benoit Daloze).
Yes, `Kernel#Time(str)` seems better than `Time.new(str)`.
However I think being precise for parsing is key for times.
Hence, why not make `Time.iso8601` fast and a core method?
Then code already using the right method would just become faster, and there is no need migrate code to use a new API.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95109
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106456] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (14 preceding siblings ...)
2021-12-03 11:33 ` [ruby-core:106452] " Eregon (Benoit Daloze)
@ 2021-12-03 13:21 ` nobu (Nobuyoshi Nakada)
2021-12-03 13:49 ` [ruby-core:106458] " nobu (Nobuyoshi Nakada)
` (8 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-12-03 13:21 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
sawa (Tsuyoshi Sawada) wrote in #note-16:
> nobu (Nobuyoshi Nakada) wrote in #note-15:
> > I remember #18331 won't work with subclasses of `Time`.
>
> Can you elaborate on that? Is your concern about certain frameworks using time-like classes other than `Time`? I don't see any problem with that. If you create a `Time` object using `Kernel.#Time` proposed in #18331, that can easily be converted (often without noticing) to such class in case of Ruby on Rails' `ActiveSupport::TimeWithZone`.
The timezone name will be converted by `find_timezone` method on the class of a `Time` instance, so that subclasses can override it.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95113
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106458] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (15 preceding siblings ...)
2021-12-03 13:21 ` [ruby-core:106456] " nobu (Nobuyoshi Nakada)
@ 2021-12-03 13:49 ` nobu (Nobuyoshi Nakada)
2021-12-06 4:55 ` [ruby-core:106501] " nobu (Nobuyoshi Nakada)
` (7 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-12-03 13:49 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
Eregon (Benoit Daloze) wrote in #note-17:
> Hence, why not make `Time.iso8601` fast and a core method?
> Then code already using the right method would just become faster, and there is no need migrate code to use a new API.
First, the primary target is the result of `Time#inspect`, and it is not fully compliant with ISO-8601.
Second, ISO-8601 allows many variants, but I'm not going to implement them all.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95115
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106501] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (16 preceding siblings ...)
2021-12-03 13:49 ` [ruby-core:106458] " nobu (Nobuyoshi Nakada)
@ 2021-12-06 4:55 ` nobu (Nobuyoshi Nakada)
2021-12-06 13:18 ` [ruby-core:106512] " Eregon (Benoit Daloze)
` (6 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-12-06 4:55 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
Eregon (Benoit Daloze) wrote in #note-10:
> Also it's technically incompatible (the first argument is always the year for `Time.new` and it even accepts strings):
> ```
> > Time.new "2234-01-01T00:00:00+07:00"
> => 2234-01-01 00:00:00 +0100
> ```
Actually this code raises an `ArgumentError` in the master, since https://bugs.ruby-lang.org/issues/17485#change-89871.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95161
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106512] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (17 preceding siblings ...)
2021-12-06 4:55 ` [ruby-core:106501] " nobu (Nobuyoshi Nakada)
@ 2021-12-06 13:18 ` Eregon (Benoit Daloze)
2021-12-06 14:54 ` [ruby-core:106516] " nobu (Nobuyoshi Nakada)
` (5 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-12-06 13:18 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by Eregon (Benoit Daloze).
nobu (Nobuyoshi Nakada) wrote in #note-19:
> First, the primary target is the result of `Time#inspect`, and it is not fully compliant with ISO-8601.
Thanks for making that clear, I wasn't sure why not just improve `Time.iso8601`.
Right, they differ:
```
irb(main):006:0> t.inspect
=> "2021-12-06 14:10:15.334531885 +0100"
irb(main):007:0> t.iso8601
=> "2021-12-06T14:10:15+01:00"
```
Why we do we need to parse `Time#inspect` output?
It seems in general bad to rely on `inspect` for serialization, since it only works for some classes.
Maybe to preserve subseconds?
`#inspect` does not feel like a proper way to serialize a Time instance, so if we want to add that I think we should have a new method for it too (maybe `#dump`?).
In any case, `Time.new` accepts individual time components, and I think making it parse strings is just confusing.
A new method `Time.try_convert` (or `Time.undump`) feels more appropriate for such parsing.
> Second, ISO-8601 allows many variants, but I'm not going to implement them all.
Is that what makes it faster?
The PR still uses a Regexp so I guess the main difference is the Regexp is a little bit simpler?
They look fairly similar:
* https://github.com/ruby/ruby/pull/4639/files
* https://github.com/ruby/ruby/blob/715a51a0d6963f9d727191d4e1ad0690fd28c4dd/lib/time.rb#L617-L650
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95171
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106516] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (18 preceding siblings ...)
2021-12-06 13:18 ` [ruby-core:106512] " Eregon (Benoit Daloze)
@ 2021-12-06 14:54 ` nobu (Nobuyoshi Nakada)
2021-12-06 17:37 ` [ruby-core:106521] " Eregon (Benoit Daloze)
` (4 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-12-06 14:54 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
Eregon (Benoit Daloze) wrote in #note-21:
> Why we do we need to parse `Time#inspect` output?
> It seems in general bad to rely on `inspect` for serialization, since it only works for some classes.
> Maybe to preserve subseconds?
I wrote `Time#inspect`, but the "ISO 8601-like" format is not only used by it, e.g., `--date=iso` of git.
> `#inspect` does not feel like a proper way to serialize a Time instance, so if we want to add that I think we should have a new method for it too (maybe `#dump`?).
> In any case, `Time.new` accepts individual time components, and I think making it parse strings is just confusing.
How about `Time.at`?
> A new method `Time.try_convert` (or `Time.undump`) feels more appropriate for such parsing.
`Time.try_convert` feels considerable, but passing the timezone option may not fit.
> > Second, ISO-8601 allows many variants, but I'm not going to implement them all.
>
> Is that what makes it faster?
Not to be faster.
ISO-8601 is a large specification and allows many omissions, which makes parsing very complex and sometimes conflicts with the spec of `Time`.
> The PR still uses a Regexp so I guess the main difference is the Regexp is a little bit simpler?
> They look fairly similar:
> * https://github.com/ruby/ruby/pull/4639/files
That PR is abandoned, please see [Time.new-string] or [Time.at-string].
[Time.new-string]: https://github.com/ruby/ruby/pull/4825
[Time.at-string]: https://github.com/ruby/ruby/pull/5212
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95177
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106521] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (19 preceding siblings ...)
2021-12-06 14:54 ` [ruby-core:106516] " nobu (Nobuyoshi Nakada)
@ 2021-12-06 17:37 ` Eregon (Benoit Daloze)
2021-12-07 14:15 ` [ruby-core:106537] " nobu (Nobuyoshi Nakada)
` (3 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-12-06 17:37 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by Eregon (Benoit Daloze).
nobu (Nobuyoshi Nakada) wrote in #note-22:
> I wrote `Time#inspect`, but the "ISO 8601-like" format is not only used by it, e.g., `--date=iso` of git.
Interesting that `git` has this too:
```
--date=iso (or --date=iso8601) shows timestamps in a ISO 8601-like format. The differences to the strict ISO 8601 format are:
• a space instead of the T date/time delimiter
• a space between time and time zone
• no colon between hours and minutes of the time zone
--date=iso-strict (or --date=iso8601-strict) shows timestamps in strict ISO 8601 format.
```
Maybe we really need a name for this. Like `Time#iso` and `Time.iso`.
Or it could be a keyword argument: `time.iso8601(format: :git)` and `Time.iso8601(str, format: :git)`.
`:git` is just an example, could be `textual`, `relaxed` or anything.
Another idea would be to actually use the strict variant of iso8601 if that is well defined, like what `git log --date=iso-strict` shows.
Then we'd have `Time#iso8601_strict`/`Time.iso8601_strict`, or `time.iso8601(format: :strict)` and `Time.iso8601(str, format: :strict)`.
That feels proper to me because then we have a clear way to serialize and deserialize times and finding the dumping/parsing method is trivial (they have the same name, no need to guess).
I feel relying on `#inspect` for efficient serializing is in general bad, because that is not the main purpose of `#inspect` (rather it's to show a clear/debug-like representation of an object).
> How about `Time.at`?
To me `Time.at` sounds like "give me a Time at epoch", so I think it's unexpected it does String parsing.
> `Time.try_convert` feels considerable, but passing the timezone option may not fit.
I think an extra optional timezone argument would be fine.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95181
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:106537] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (20 preceding siblings ...)
2021-12-06 17:37 ` [ruby-core:106521] " Eregon (Benoit Daloze)
@ 2021-12-07 14:15 ` nobu (Nobuyoshi Nakada)
2022-12-15 6:11 ` [ruby-core:111293] " mame (Yusuke Endoh)
` (2 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-12-07 14:15 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by nobu (Nobuyoshi Nakada).
Eregon (Benoit Daloze) wrote in #note-23:
> > `Time.try_convert` feels considerable, but passing the timezone option may not fit.
>
> I think an extra optional timezone argument would be fine.
Ah, my bad.
The `try_convert` methods are the implicit conversions for duck-typing, i.e, `to_int` to `Integer`, `to_str` to `String` and so on.
As parsing like `Kernel#Integer` is not an implicit conversion, `Time.try_convert` can't be valid.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-95203
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:111293] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (21 preceding siblings ...)
2021-12-07 14:15 ` [ruby-core:106537] " nobu (Nobuyoshi Nakada)
@ 2022-12-15 6:11 ` mame (Yusuke Endoh)
2022-12-15 6:54 ` [ruby-core:111295] " matz (Yukihiro Matsumoto)
2024-10-11 12:49 ` [ruby-core:119511] " Eregon (Benoit Daloze) via ruby-core
24 siblings, 0 replies; 26+ messages in thread
From: mame (Yusuke Endoh) @ 2022-12-15 6:11 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by mame (Yusuke Endoh).
https://github.com/ruby/ruby/pull/4825
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-100651
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4639
--
https://bugs.ruby-lang.org/
______________________________________________
ruby-core mailing list -- ruby-core@ml.ruby-lang.org
To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:111295] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (22 preceding siblings ...)
2022-12-15 6:11 ` [ruby-core:111293] " mame (Yusuke Endoh)
@ 2022-12-15 6:54 ` matz (Yukihiro Matsumoto)
2024-10-11 12:49 ` [ruby-core:119511] " Eregon (Benoit Daloze) via ruby-core
24 siblings, 0 replies; 26+ messages in thread
From: matz (Yukihiro Matsumoto) @ 2022-12-15 6:54 UTC (permalink / raw)
To: ruby-core
Issue #18033 has been updated by matz (Yukihiro Matsumoto).
Accepted.
Performance improvement is a great benefit.
Both `Time.iso8859` and `Time.parse` requires loading `date` gem, and providing those methods in the core may cause overwriting.
Currently, `Time.new` only takes numeric arguments, so adding extra argument pattern add only a small complexity.
Matz.
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-100654
* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4825
--
https://bugs.ruby-lang.org/
______________________________________________
ruby-core mailing list -- ruby-core@ml.ruby-lang.org
To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ruby-core:119511] [Ruby master Feature#18033] Time.new to parse a string
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
` (23 preceding siblings ...)
2022-12-15 6:54 ` [ruby-core:111295] " matz (Yukihiro Matsumoto)
@ 2024-10-11 12:49 ` Eregon (Benoit Daloze) via ruby-core
24 siblings, 0 replies; 26+ messages in thread
From: Eregon (Benoit Daloze) via ruby-core @ 2024-10-11 12:49 UTC (permalink / raw)
To: ruby-core; +Cc: Eregon (Benoit Daloze)
Issue #18033 has been updated by Eregon (Benoit Daloze).
matz (Yukihiro Matsumoto) wrote in #note-27:
> Currently, `Time.new` only takes numeric arguments, so adding extra argument pattern add only a small complexity.
But older versions like 3.0 accept year as a String: https://bugs.ruby-lang.org/issues/18033#note-10
So we get very weird behavior between versions:
```
$ ruby -ve 'p Time.new "2234-10-12T01:02:03"'
ruby 3.0.6p216 (2023-03-30 revision 23a532679b) [x86_64-linux]
2234-01-01 00:00:00 +0100
$ ruby -ve 'p Time.new "2234-10-12T01:02:03"'
ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x86_64-linux]
<internal:timev>:310:in `initialize': invalid value for Integer(): "2234-10-12T01:02:03" (ArgumentError)
from -e:1:in `new'
from -e:1:in `<main>'
$ ruby -ve 'p Time.new "2234-10-12T01:02:03"'
ruby 3.2.4 (2024-04-23 revision af471c0e01) [x86_64-linux]
2234-10-12 01:02:03 +0200
$ ruby -ve 'p Time.new "2234-10-12T01:02:03"'
ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-linux]
2234-10-12 01:02:03 +0200
```
----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-110126
* Author: nobu (Nobuyoshi Nakada)
* Status: Closed
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.
* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.
```
$ ./ruby -rtime -rbenchmark -e '
n = 1000
s = Time.now.iso8601
Benchmark.bm(12) do |x|
x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
x.report("Time.parse") {n.times{Time.parse(s)}}
x.report("Time.new") {n.times{Time.new(s)}}
end'
user system total real
Time.iso8601 0.006919 0.000185 0.007104 ( 0.007091)
Time.parse 0.018338 0.000207 0.018545 ( 0.018590)
Time.new 0.003671 0.000069 0.003740 ( 0.003741)
```
https://github.com/ruby/ruby/pull/4825
--
https://bugs.ruby-lang.org/
______________________________________________
ruby-core mailing list -- ruby-core@ml.ruby-lang.org
To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2024-10-11 12:50 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-09 6:13 [ruby-core:104552] [Ruby master Feature#18033] Time.new to parse a string nobu
2021-07-09 6:30 ` [ruby-core:104554] " samuel
2021-07-09 7:51 ` [ruby-core:104559] " nobu
2021-07-09 8:29 ` [ruby-core:104562] " samuel
2021-07-09 8:51 ` [ruby-core:104564] " nobu
2021-07-10 8:42 ` [ruby-core:104575] " jean.boussier
2021-07-16 2:48 ` [ruby-core:104626] " samuel
2021-10-24 23:14 ` [ruby-core:105763] " ioquatix (Samuel Williams)
2021-11-10 10:44 ` [ruby-core:106002] " Eregon (Benoit Daloze)
2021-11-10 11:26 ` [ruby-core:106007] " byroot (Jean Boussier)
2021-11-12 11:33 ` [ruby-core:106036] " timcraft (Tim Craft)
2021-12-03 4:07 ` [ruby-core:106440] " nobu (Nobuyoshi Nakada)
2021-12-03 6:54 ` [ruby-core:106443] " nobu (Nobuyoshi Nakada)
2021-12-03 10:08 ` [ruby-core:106449] " sawa (Tsuyoshi Sawada)
2021-12-03 10:23 ` [ruby-core:106451] " sawa (Tsuyoshi Sawada)
2021-12-03 11:33 ` [ruby-core:106452] " Eregon (Benoit Daloze)
2021-12-03 13:21 ` [ruby-core:106456] " nobu (Nobuyoshi Nakada)
2021-12-03 13:49 ` [ruby-core:106458] " nobu (Nobuyoshi Nakada)
2021-12-06 4:55 ` [ruby-core:106501] " nobu (Nobuyoshi Nakada)
2021-12-06 13:18 ` [ruby-core:106512] " Eregon (Benoit Daloze)
2021-12-06 14:54 ` [ruby-core:106516] " nobu (Nobuyoshi Nakada)
2021-12-06 17:37 ` [ruby-core:106521] " Eregon (Benoit Daloze)
2021-12-07 14:15 ` [ruby-core:106537] " nobu (Nobuyoshi Nakada)
2022-12-15 6:11 ` [ruby-core:111293] " mame (Yusuke Endoh)
2022-12-15 6:54 ` [ruby-core:111295] " matz (Yukihiro Matsumoto)
2024-10-11 12:49 ` [ruby-core:119511] " Eregon (Benoit Daloze) via ruby-core
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).