-
Notifications
You must be signed in to change notification settings - Fork 64
Document format function #1374
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
Document format function #1374
Conversation
…acters tables, added more headings
JoelBergstrand
left a comment
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.
Great work! I'd like an update to the toString page, or where you think it would be relevant, that temporal values by default output on the ISO format. It should be clear to the user that you don't need to use the format function to get ISO.
| ---- | ||
| WITH datetime('1986-11-18T6:04:45.123456789+01:00[Europe/Berlin]') AS dt | ||
| RETURN format(dt, "yyyy-MM-dd'T'HH:mm:ss.SSSZ") AS x | ||
| ---- | ||
| .Result | ||
| [role="queryresult",options="header,footer",cols="1*<m"] | ||
| |=== | ||
| | x | ||
| | "1986-11-18T06:04:45.123+0100" | ||
| 1+d|Rows: 1 |
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 don't think we should lead with this test as it return the ISO representation of the datetime, e.g. it doesn't do anything new and useful.
| If the string pattern contains a character from a component group but does not contain a character denoting a longer duration from the same group, `format()` converts the longer duration to the equivalent duration with the character that is present, for example a missing `y` (year) will be converted to four quarters, if `q` is present in the string pattern. | ||
| This is because without a reference point, there is no way of determining the specifics of a duration. | ||
|
|
||
| For example, not all months have the same number of days, and switches to or from daylight saving time means that a day has 23 or 25 hours rather than 24. |
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 wonder if this should be omitted as we discuss this in components of durations
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.
only line 262, or the entire introduction?
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.
Line 262 is my thinking, the reset it format specfic information
|
|
||
|
|
||
| [[query-functions-temporal-format-duration-types-characters]] | ||
| === String pattern characters |
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.
Looks like there might be a line too much after this title
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.
the spacing in the preview?
i think this might be because of table title 🤔 it's correct in the source document (or rather, i'm pretty sure it wouldn't make a difference in this case)
| [[query-functions-temporal-format-duration-types-examples]] | ||
| === Examples | ||
|
|
||
| .Duration formatting, years converted to quarters |
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.
For all these test we use the full duration constructor, even when test don't use all component groups. As the map is big and ugly, do we want it in all tests? Or a smaller map with only relevant values?
| .Query | ||
| [source, cypher, indent=0] | ||
| ---- | ||
| WITH datetime('1986-11-18T6:04:45.123456789+01:00[Europe/Berlin]') AS dt | ||
| RETURN format(dt, "yyyy-MM-dd'T'HH:mm:ss.SSSZ") AS x | ||
| ---- | ||
| .Result | ||
| [role="queryresult",options="header,footer",cols="1*<m"] | ||
| |=== | ||
| | x | ||
| | "1986-11-18T06:04:45.123+0100" | ||
| 1+d|Rows: 1 |
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'd switch this to the EU -> US example.
|
|
||
| The format function creates dynamically formatted string representations of temporal instance and duration types. | ||
| The output format can be customized via the `pattern` parameter. | ||
| Creating a pattern follows the rules for the link:https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html[Java DateTimeFormatter]. |
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.
This links to Java 8, but we use 21:
https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/time/format/DateTimeFormatter.html
| .Details | ||
| |=== | ||
| | *Syntax* 3+| `format(value[, pattern])` | ||
| | *Description* 3+| Returns the temporal value as an ISO-formatted string or as a string formatted by the provided pattern. |
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.
Don't we normally say STRING etc in these descriptions?
| |=== | ||
| | Most characters yield a different output when they are repeated. | ||
| | Some characters cannot be applied to certain types, for instance, `u` cannot be used to construct a string for a `LOCAL TIME` because it represents a year which is not part of the temporal value. | ||
| | For details, refer to the documentation of the link:https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html[Java DateTimeFormatter]. |
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.
Co-authored-by: Gem Lamont <106068376+gem-neo4j@users.noreply.github.com>
JPryce-Aklundh
left a comment
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.
Looks good! My main suggestion would be to put the Considerations table immediately after the Details table (as with the other functions), and to put most of the details from your introductory paragraph in the Considerations table.
modules/ROOT/pages/deprecations-additions-removals-compatibility.adoc
Outdated
Show resolved
Hide resolved
| |=== | ||
| | Most characters yield a different output when they are repeated. | ||
| | Some characters cannot be applied to certain types, for instance, `u` cannot be used to construct a string for a `LOCAL TIME` because it represents a year which is not part of the temporal value. | ||
| | For details, refer to the documentation of the link:https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/time/format/DateTimeFormatter.html[Java DateTimeFormatter]. |
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.
Do we need this link twice?
Usually, the Considerations points come just after the Description table, and I think just having the link in the Considerations table suffices.
| |=== | ||
| Note how the days cannot be converted to hours and do not affect the query result. |
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.
Im not sure I follow here? Can you add a short explanation?
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.
that was gibberish indeed - the example did not match at all what i wanted to point out. updated 👍
Co-authored-by: Jens Pryce-Åklundh <112686610+JPryce-Aklundh@users.noreply.github.com>
JoelBergstrand
left a comment
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.
Great!
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.
Good stuff! I have a few more suggestions.
Also, format() needs to be added to its own table on the Functions overview page:
https://neo4j.com/docs/cypher-manual/25/functions/
In that table, please add the labels, label:new[Introduced in Neo4j 2025.09] label:cypher[Cypher 25 only]
| |=== | ||
|
|
||
| [[query-functions-temporal-format-instance-types]] | ||
| == Instance types |
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.
Instance types and duration types being H2 (==) might become confusing navigation wise if more format functions are introduced (usually the functions themselves are on the H2 level).
I'll leave it up to you change for now or not (and so long as there is only 1, I think its fine to leave as is), but just thought I'd point it out :)
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'd say let's leave it for now, but definitely address it when we add more.
heading levels shouldn't interfere with anchored links either, so this sort of refactoring ought to be safe
| |=== | ||
| | instanceString | ||
| | "322th day of the year, 3rd day of the week" |
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 know it is just an example, but it bothers me that we don't say 322nd 😅
Co-authored-by: Jens Pryce-Åklundh <112686610+JPryce-Aklundh@users.noreply.github.com>
Co-authored-by: Jens Pryce-Åklundh <112686610+JPryce-Aklundh@users.noreply.github.com>
JPryce-Aklundh
left a comment
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.
🎉
modules/ROOT/pages/deprecations-additions-removals-compatibility.adoc
Outdated
Show resolved
Hide resolved
|
Thanks for the documentation updates. The preview documentation has now been torn down - reopening this PR will republish it. |
No description provided.