Implement addition and subtraction for Date_Period and Time_Period #6956
Conversation
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Time.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Time_Of_Day.enso
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Agree with @radeusgd comments - but lets also turn on the runtime type checking in a few places.
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date.enso
Outdated
Show resolved
Hide resolved
| - amount: The amount of time to add to this instant, either `Duration` for | ||
| adding time (hours, minutes, etc.), or `Period` for adding date (days, | ||
| months, years). | ||
| adding time (hours, minutes, etc.), `Period` for adding date (days, | ||
| months, years), or `Date_Period` for adding a single unit (year, | ||
| quarter, month, etc.). |
There was a problem hiding this comment.
Should have Time_Period as @radeusgd says. Probaly not "either" if 4 options.
Might be easier to just lose the stuff after this instant.
There was a problem hiding this comment.
How about just:
- amount: The amount of time to add to this instant, It can be
`Duration`, `Period`, `Date_Period`, or `Time_Period`.
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Time.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Time.enso
Outdated
Show resolved
Hide resolved
| @@ -430,8 +430,8 @@ type Date | |||
| Add the specified amount of time to this instant to get another date. | |||
|
|
|||
| Arguments: | |||
| - amount: The time duration to add to this instant. Can only be a | |||
| `Period` or a `Date_Period`. | |||
| - amount: The amount of time to add to this instant. It can an be | |||
There was a problem hiding this comment.
There's something wrong with this sentence. Please fix wording.
There was a problem hiding this comment.
Done, made the comments uniform.
| + : Period -> Date ! (Time_Error | Illegal_Argument) | ||
| + self amount = | ||
| + : Period | Date_Period -> Date ! (Time_Error | Illegal_Argument) | ||
| + self amount:(Period|Date_Period) = |
There was a problem hiding this comment.
Personally I prefer
| + self amount:(Period|Date_Period) = | |
| + self (amount : Period|Date_Period) = |
as IMO it looks nicer. But that is very subjective, feel free to ignore, just sharing my preferences :)
There was a problem hiding this comment.
@jdunkerley what do you think?
| @@ -370,6 +370,19 @@ spec_with name create_new_datetime parse_datetime nanoseconds_loss_in_precision= | |||
| time+Date_Period.Quarter-Date_Period.Month . should_equal <| create_new_datetime 1970 3 1 (zone = Time_Zone.utc) | |||
| time+Date_Period.Day-Time_Period.Day . should_equal <| create_new_datetime 1970 (zone = Time_Zone.utc) | |||
|
|
|||
| Test.specify "should reflect that Time_Period.Day does not reflect daylight saving" <| | |||
There was a problem hiding this comment.
minor nitpick
| Test.specify "should reflect that Time_Period.Day does not reflect daylight saving" <| | |
| Test.specify "will reflect that Time_Period.Day does not reflect daylight saving" <| |
for tests that show behaviour which is not necessarily what we desire but they document what is currently implemented, I tend to go for will instead of should; to distinguish such tests. IMO this gives us a bit more precision to what is the intended feature and what is more of an 'accidental' one
GregoryTravis
added
the
CI: Ready to merge
Pull Request Description
Date.+ should allow Date_Period, Time_Of_Day.+ should allow Time_Period and Date_TIme.+ should allow both.
Same for subtraction.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.