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

add paragraph in documentation that explains why Time::HiRes::time shoul... #6

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@reneeb

reneeb commented Jan 2, 2015

...d be sprintf'ed for from_epoche

time returns 15 digits by default. So the floating point portion can't have 9 digits (as needed for nanoseconds). The solution is to
use sprintf.

add paragraph in documentation that explains why Time::HiRes::time sh…
…ould be sprintf'ed for from_epoche

time returns 15 digits by default. So the floating point portion can't have 9 digits (as needed for nanoseconds). The solution is to
use sprintf.
@eserte

This comment has been minimized.

Show comment
Hide comment
@eserte

eserte Jan 2, 2015

See the discussion in https://rt.cpan.org/Ticket/Display.html?id=96452
I disagree that this is a documentation issue; DateTime can be smarter here.

eserte commented Jan 2, 2015

See the discussion in https://rt.cpan.org/Ticket/Display.html?id=96452
I disagree that this is a documentation issue; DateTime can be smarter here.

@autarch

This comment has been minimized.

Show comment
Hide comment
@autarch

autarch Jan 28, 2015

Collaborator

I'd be up for an interface that accepted the return value of Time::HiRes::gettimeofday, if that's what you're referring to, @eserte. Maybe it makes sense to make DateTime->from_epoch( epoch => gettimeofday() ) work?

Collaborator

autarch commented Jan 28, 2015

I'd be up for an interface that accepted the return value of Time::HiRes::gettimeofday, if that's what you're referring to, @eserte. Maybe it makes sense to make DateTime->from_epoch( epoch => gettimeofday() ) work?

@reneeb

This comment has been minimized.

Show comment
Hide comment
@reneeb

reneeb Jan 28, 2015

IMHO DateTime->from_epoch( epoch => gettimeofday() ) is not very nice as from_epoch would get three parameters: "epoch", seconds and microseconds. gettimeofday is context-sensitive. The way you showed it, it would return two elements and it is not the same as

my $time_of_day = gettimeofday();
DateTime->from_epoch( epoch => $time_of_day );

reneeb commented Jan 28, 2015

IMHO DateTime->from_epoch( epoch => gettimeofday() ) is not very nice as from_epoch would get three parameters: "epoch", seconds and microseconds. gettimeofday is context-sensitive. The way you showed it, it would return two elements and it is not the same as

my $time_of_day = gettimeofday();
DateTime->from_epoch( epoch => $time_of_day );
@autarch

This comment has been minimized.

Show comment
Hide comment
@autarch

autarch Jan 28, 2015

Collaborator

Ah, I didn't realize it was context-sensitive. I would only want to allow the epoch param to be either a single number or a 2-element arrayref.

Collaborator

autarch commented Jan 28, 2015

Ah, I didn't realize it was context-sensitive. I would only want to allow the epoch param to be either a single number or a 2-element arrayref.

@eserte

This comment has been minimized.

Show comment
Hide comment
@eserte

eserte Jan 30, 2015

One possibility would be to use

    DateTime->from_epoch( epoch => [Time::HiRes::gettimeofday] );

It's not very unusual to wrap a gettimeofday() call in square brackets; you do this also if you want to measure durations with Time::HiRes::tv_interval.

If a user forgets the square brackets, then he'll get an error like this (currently and also in future):

Odd number of parameters in call to DateTime::from_epoch when named parameters were expected
 at /usr/local/lib/perl5/site_perl/5.16/mach/DateTime.pm line 517.
        DateTime::from_epoch(undef, "epoch", 1422646510, 506309) called at /tmp/dt.pl line 4

This is rather cryptic. maybe DateTime can detect here that the key without value is an integer and give better diagnostics, e.g. "Did you mean DateTime->from_epoch(epoch => [1422646510, 506309]), that is, wrapping the argument in an array reference?"

eserte commented Jan 30, 2015

One possibility would be to use

    DateTime->from_epoch( epoch => [Time::HiRes::gettimeofday] );

It's not very unusual to wrap a gettimeofday() call in square brackets; you do this also if you want to measure durations with Time::HiRes::tv_interval.

If a user forgets the square brackets, then he'll get an error like this (currently and also in future):

Odd number of parameters in call to DateTime::from_epoch when named parameters were expected
 at /usr/local/lib/perl5/site_perl/5.16/mach/DateTime.pm line 517.
        DateTime::from_epoch(undef, "epoch", 1422646510, 506309) called at /tmp/dt.pl line 4

This is rather cryptic. maybe DateTime can detect here that the key without value is an integer and give better diagnostics, e.g. "Did you mean DateTime->from_epoch(epoch => [1422646510, 506309]), that is, wrapping the argument in an array reference?"

@autarch

This comment has been minimized.

Show comment
Hide comment
@autarch

autarch Feb 1, 2015

Collaborator

@eserte - that seems pretty reasonable.

Collaborator

autarch commented Feb 1, 2015

@eserte - that seems pretty reasonable.

@reneeb

This comment has been minimized.

Show comment
Hide comment
@reneeb

reneeb Feb 9, 2015

Another issue with gettimeofday: It returns microseconds, not nanoseconds... What should the second element treated as? microseconds, nanoseconds? And what to do when the user passes more digits than "allowed" (e.g. 10 digits when we we want nanoseconds)?

reneeb commented Feb 9, 2015

Another issue with gettimeofday: It returns microseconds, not nanoseconds... What should the second element treated as? microseconds, nanoseconds? And what to do when the user passes more digits than "allowed" (e.g. 10 digits when we we want nanoseconds)?

@eserte

This comment has been minimized.

Show comment
Hide comment
@eserte

eserte Feb 9, 2015

This could be "fixed" with different naming for the parameter, e.g. by using DateTime->from_epoch(gettimeofday => [gettimeofday]) or alternatively DateTime->from_epoch(s_ms => [gettimeofday])

eserte commented Feb 9, 2015

This could be "fixed" with different naming for the parameter, e.g. by using DateTime->from_epoch(gettimeofday => [gettimeofday]) or alternatively DateTime->from_epoch(s_ms => [gettimeofday])

@chansen

This comment has been minimized.

Show comment
Hide comment
@chansen

chansen Sep 19, 2015

Contributor

I have submitted a PR which changes ->from_epoch, #11

Contributor

chansen commented Sep 19, 2015

I have submitted a PR which changes ->from_epoch, #11

@autarch

This comment has been minimized.

Show comment
Hide comment
@autarch

autarch May 13, 2016

Collaborator

Is this still relevant in light of @chansen's changes in #11?

Collaborator

autarch commented May 13, 2016

Is this still relevant in light of @chansen's changes in #11?

@chansen chansen referenced this pull request May 18, 2016

Closed

From epoch #15

@reneeb

This comment has been minimized.

Show comment
Hide comment
@reneeb

reneeb May 20, 2016

I think it can be closed...

reneeb commented May 20, 2016

I think it can be closed...

@reneeb reneeb closed this May 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment