apjanke / octave-tablicious

Table (relational, tabular data) implementation for GNU Octave
https://apjanke.github.io/octave-tablicious/
GNU General Public License v3.0
28 stars 11 forks source link

Mixed-type duration math: addition of calyears or calmonths with days, hours, etc. #136

Open jgpallero opened 3 weeks ago

jgpallero commented 3 weeks ago

I can see that duration objects and calendarDuration objects can't be added or substracted

>> calyears(8)+hours(8)
error: Invalid input: B must be numeric or calendarDuration; got a duration
error: called from
    plus at line 248 column 9

In Matlab this kind of operation works and the result can be used as increment with the colon operator in order to create sequences. Is this a bug in tablicious or a feature?

apjanke commented 3 weeks ago

In Matlab this kind of operation works and the result can be used as increment with the colon operator in order to create sequences. Is this a bug in tablicious or a feature?

It's a bug. Tablicious's date/time support is only like 60% complete, with respect to Matlab's full functionality. :)

I'll prioritize adding this in, now that I know someone actually wants to use it. Can probably have it done and out in a new Tablicious 0.4.4 release by, say, Thursday?

Diagnosis

I think the issue here is that addition is defined for the duration and calendarDuration classes, but not mixed-type addition between them.

>> calyears(2) + calyears(2)
ans =
4y
>> calyears(2) + calmonths(3)
ans =
2y 3mo
>> class(hours(3))
ans = duration
>> hours(3) + hours(2)
ans =
05:00:00
>> hours(3) + minutes(2)
ans =
03:02:00

And calendarDuration has a times method, but I missed the mtimes variant.

>> calyears(3) * 3
error: mtimes method not defined for calendarDuration class
>> calyears(3) .* 3
ans =
9y
>> hours(3) * 3
ans =
09:00:00
>> hours(3) .* 3
ans =
09:00:00
>>

Both of these fixes should be pretty straightforward. Might be complicated a bit by the need to support mixed signs between the different fields, and handle how calendar years and months get normalized between each other.

jgpallero commented 2 weeks ago

For completeness, in calendarDuration class there is not defined the mtimes method in order to multiply calyears and calmonths times a scalar. This operation is defined in Matlab

apjanke commented 2 weeks ago

That's in now, in https://github.com/apjanke/octave-tablicious/commit/3f5fbc7b27050016ad45e5fcb4fc14dfb21ffd88 on branch caldur-math-fixes.

>> calyears(2) * 3
ans =
6y
>> calmonths(2) * 3
ans =
6mo
>>
apjanke commented 2 weeks ago

Here's a fix for mixed-type duration + calendarDuration arithmetic, I think: https://github.com/apjanke/octave-tablicious/commit/b8e3745c1a29583da0927b863bf6f2c369fd90fe

>> calmonths(1) + days(3.4)
ans =
1mo 3 days 09:36:00
>> days(3.4) + calmonths(1)
ans =
1mo 3 days 09:36:00
>>
jgpallero commented 2 weeks ago

Thank you very much for your (date)time :)

apjanke commented 2 weeks ago

LOL. :)