-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Adds DateTime[Immutable]::createFromTimestamp #12413
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
Conversation
fcd3402
to
95e12ab
Compare
…le]_from_timestamp
95e12ab
to
29eca46
Compare
1ac0743
to
c7cc823
Compare
d517b20
to
c7cc823
Compare
ext/date/php_date.stub.php
Outdated
@@ -362,6 +368,12 @@ public static function createFromInterface(DateTimeInterface $object): DateTime | |||
*/ | |||
public static function createFromFormat(string $format, string $datetime, ?DateTimeZone $timezone = null): DateTime|false {} | |||
|
|||
/** | |||
* @tentative-return-type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tentative-return-type
makes Carbon compatible with deprecation message
The signatures you listed do not contain |
@kamil-tekiela Thanks for pointing this out. It has been updated already to throw a |
@derickr I have updated the PR according to your review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good, but I have one question left.
["timezone"]=> | ||
string(6) "+00:00" | ||
} | ||
DateTime::createFromTimestamp(%f): DateRangeError: Seconds must be a finite number between %i and %i, %f given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you say finite
here, and not integer
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the word "finite" to cover NAN, +/-INF as well and as a FP is allowed as well "integer" would not be fully correct.
If you feel it's confusing it could also be left out -> "... must be a number between ..."
I have merged this after a rebase, squash, and the addition of the NEWS entry. Thanks! |
@marc-mabe Turns out that this failed tests on x32, can you have a look? https://github.com/php/php-src/actions/runs/6964523973/job/18951903197 |
I'll have a look ... Could you give me a hint for how I can test this without x32 arch 🙏 |
I run compile my 32-bit build like this: https://github.com/derickr/php-build/blob/master/build#L68 You can probably get away with just I guess, this would be minimal:
|
…omTimestamp() (derrabus) This PR was merged into the 7.1 branch. Discussion ---------- [Clock] Add a polyfill for DateTimeImmutable::createFromTimestamp() | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | N/A | License | MIT This PR adds the new `createFromTimestamp()` method introduced in php/php-src#12413 to our `DatePoint` class. It comes with a polyfill implementation for PHP 8.2/8.3. Commits ------- 9e48e0d [Clock] Polyfill DateTimeImmutable::createFromTimestamp()
I'm late to the party but I'm wondering why in contrast to |
To answer my own question, |
Because a UNIX timestamp already contains a time zone. Specifying a time zone would be redundant as you have found out yourself. |
This creates the following functions:
The reason for these functions is the very common case of creating a DateTime[Immutable] object from a unix timestamp.
This currently only works by:
@<timestamp>
createFromFormat
withU
/U.u
@<timstamp>
but adds additional differences on integer vs. float(new DateTime(...))->setTimestamp()
setTimestamp
supports integers only (This probably should allow floats as well in a separate PR)The new functions avoid the above issues, are self explained and are much faster for this common case as well.
The functions will throw a
DateRangeError
in case of special floating point numbers or out-of-range of signed long long of timelib.Internals: https://marc.info/?l=php-internals&m=169850186312326
Bench (doesn't even handle scientific notation of floating points):