Bug 88547 - FILEOPEN:Functions NETWORKDAYS.INTL WORKDAY.INTL incompatibility with excel
Summary: FILEOPEN:Functions NETWORKDAYS.INTL WORKDAY.INTL incompatibility with excel
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.3.0.4 release
Hardware: Other All
: medium normal
Assignee: Winfried Donkers
URL:
Whiteboard: target:5.0.0
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-17 22:27 UTC by raal
Modified: 2015-04-21 08:28 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
test case (37.95 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2015-01-17 22:27 UTC, raal
Details
expanded testcase (32.70 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2015-03-11 10:40 UTC, Winfried Donkers
Details
comparison Excel2010 and Calc (with modified code) (14.75 KB, application/vnd.oasis.opendocument.spreadsheet)
2015-03-19 12:29 UTC, Winfried Donkers
Details
expanded testcase (5.98 KB, application/zip)
2015-04-21 08:27 UTC, Winfried Donkers
Details

Note You need to log in before you can comment on or make changes to this bug.
Description raal 2015-01-17 22:27:39 UTC
Created attachment 112407 [details]
test case

In bug 73147 was added NETWORKDAYS.INTL and WORKDAY.INTL spreadsheet functions for Excel interoperability.

Steps to reproduce
- open attachment
- press CTRL+SHIFT+F9 (hard recalc)

Actual results:
A1:A5 = #VALUE!

Expected results
22
22

38748
38748

Printscreens in bugfile.
Comment 1 m_a_riosv 2015-01-18 03:08:24 UTC
Hi @raal,

seems that the problem is with the in line array values for holidays, making reference to a range for holidays seems works fine. (LibreOffice Version: 4.4.0.2)

In any case there is a bug.
Comment 2 MM 2015-01-18 12:54:58 UTC
Is this the same as https://bugs.freedesktop.org/show_bug.cgi?id=77985 ???
Comment 3 Winfried Donkers 2015-01-18 13:12:02 UTC
Seems there are two problems:
1. the function does not work with inline arrays for holidays;
2. the date format used in the inline array for holidays is not recognised.

Problem 1 is clearly a problem of the function NETWORKDAYS.INTL, of problem 2 I'm not sure.
Comment 4 Winfried Donkers 2015-01-18 13:14:33 UTC
(In reply to MM from comment #2)
> Is this the same as https://bugs.freedesktop.org/show_bug.cgi?id=77985 ???

No, it's not.
This bug report regards Excel function NETWORKDAYS.INTL, bug #77985 is about Calc function NETWORKDAYS.
Comment 5 Commit Notification 2015-01-20 14:22:43 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=104a1e641554be2e789758ae67c0e24620df8035

fdo#88547 allow inline date-arrays for array functions

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 6 Eike Rathke 2015-01-20 14:24:15 UTC
For 2., "2006/1/2" is a locale dependent date format, it may or may not work in other locales. To make it work in any locale use the ISO 8601 date format instead, "2006-1-2"
Comment 7 Winfried Donkers 2015-01-20 16:12:03 UTC
(In reply to Eike Rathke from comment #6)
> For 2., "2006/1/2" is a locale dependent date format, it may or may not work
> in other locales. To make it work in any locale use the ISO 8601 date format
> instead, "2006-1-2"

I had come to that conclusion too. 
It may happen between two Calc-users or Excel-users, when there locale is different. The inline array is stored as a text-array, not interpreted as a date array.
Dates entered in cells are interpreted as dates and stored as dates and so have no problem with locale settings.
Comment 8 m_a_riosv 2015-01-20 22:38:51 UTC
(In reply to Winfried Donkers from comment #7)
> different. The inline array is stored as a text-array, not interpreted as a
> date array.
> Dates entered in cells are interpreted as dates and stored as dates and so
> have no problem with locale settings.

I think using DATE() function instead text in the inline array can avoid local settings issues.
=NETWORKDAYS.INTL(DATE(2006;1;1);DATE(2006;2;1);7;{DATE(2006;1;2);DATE(2006;1;16)})
Comment 9 raal 2015-01-21 22:05:18 UTC
Version: 4.5.0.0.alpha0+
Build ID: 60143f4f7bc50054dcef923218b8c7c3bc154933
TinderBox: Linux-rpm_deb-x86_64@46-TDF, Branch:master, Time: 2015-01-21_04:58:34

Import, export OK. Thanks for the fix.
Comment 10 Commit Notification 2015-03-09 13:12:16 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=18de9b1de1aa404b3ca12e2018e5e5e220dd9786

Revert "fdo#88547 allow inline date-arrays for array functions", tdf#89872

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2015-03-09 13:24:02 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=18de9b1de1aa404b3ca12e2018e5e5e220dd9786

Revert "fdo#88547 allow inline date-arrays for array functions", tdf#89872

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2015-03-09 13:27:05 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3feb8d18cfc7620891976c7fe116988a57192b79&h=libreoffice-4-4

Revert "fdo#88547 allow inline date-arrays for array functions", tdf#89872

It will be available in 4.4.2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2015-03-09 13:29:47 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=18de9b1de1aa404b3ca12e2018e5e5e220dd9786

Revert "fdo#88547 allow inline date-arrays for array functions", tdf#89872

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2015-03-09 13:32:15 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=18de9b1de1aa404b3ca12e2018e5e5e220dd9786

Revert "fdo#88547 allow inline date-arrays for array functions", tdf#89872

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2015-03-09 13:34:20 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=18de9b1de1aa404b3ca12e2018e5e5e220dd9786

Revert "fdo#88547 allow inline date-arrays for array functions", tdf#89872

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 16 Eike Rathke 2015-03-09 13:36:56 UTC
Reopening because of reverted change.
Comment 17 Winfried Donkers 2015-03-11 10:40:11 UTC
Created attachment 114038 [details]
expanded testcase

Further investigation produced some interesting results:
-not only NETWORKDAYS.INTL and WORKDAY.INTL are affected, but MEDIAN, MODE.MULT, FREQUENCY, LARGE, SMALL, MODALVALUE, PERCENTRANK, RANK, TRIMMEAN, PERCENTILE and QUARTILE as well.
-my fix made inline text arrays with dates work, but inline arrays with DATE(yr,mon,day) doesn't work yet.

I expanded the test case with some other functions as well.

@raal: 
Could you verify that column D reflects the results from Excel?
Thank you :-)
Comment 18 Winfried Donkers 2015-03-11 11:31:25 UTC
Attachment https://bugs.documentfoundation.org/attachment.cgi?id=114039 to bug 89872 will be useful for testing a fix for this bug without breaking the fix for bug 89872.
Comment 19 Winfried Donkers 2015-03-12 11:59:32 UTC
(In reply to Winfried Donkers from comment #17)
> -my fix made inline text arrays with dates work, but inline arrays with
> DATE(yr,mon,day) doesn't work yet.

Neither do they work in Excel, so no (immediate) action required here.
Comment 20 Winfried Donkers 2015-03-19 12:29:06 UTC
Created attachment 114188 [details]
comparison Excel2010 and Calc (with modified code)

This document shows variations of inline array use for all Calc functions using ScInterpreter::GetNumberSequenceArray() plus some other functions.
The document has been created with Calc, modified to accept inline arrays containing text strings that can be converted to numeric values. The results of the functions are copied in column B and C.
The document (xlsx export) has been tested with Excel 2010, thanks to raal, who copied the Excel results in column D.

The comparison shows that Excel seems to be inconsistent with respect to inline arrays containing text strings that can be converted to numeric values. All array variations are accepted by Excel, but not in all functions and in some functions not at all.

My suggestion is to have Calc accept inline arrays containing text strings that can be converted to numeric values for all functions using ScInterpreter::GetNumberSequenceArray().

As an extra I notice that inline arrays with functions (e.g. {DATE(...);DATE(...)} are not accepted. It would be nice if they would.
Comment 21 Eike Rathke 2015-03-24 10:54:03 UTC
How did you obtain the "Calc result" values? For me almost all formulas return #VALUE! errors. Was that produced using the modified code?

And please don't use "2-1-2006" inline dates that work only in specific locales, use ISO 8601 dates instead, e.g. "2006-1-2", that should be recognized in all locales.

However, for functions where Excel and Calc so far produce identical results this illustrates that indeed the number sequence does not include strings converted to numbers, and GetNumberSequenceArray() should not be changed.
Comment 22 Winfried Donkers 2015-03-24 11:16:46 UTC
(In reply to Eike Rathke from comment #21)
> How did you obtain the "Calc result" values? For me almost all formulas
> return #VALUE! errors. Was that produced using the modified code?
I used the patch in gerrit 14839 to produce the calc results.

> However, for functions where Excel and Calc so far produce identical results
> this illustrates that indeed the number sequence does not include strings
> converted to numbers, and GetNumberSequenceArray() should not be changed.
With one exception: RANK.

http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part2.html#__RefHeading__1018894_715980110 led me to believe that text in inline arrays ought to be supported.
But I can of course just make NETWORKDAYS.INTL, WORKDAY.INTL and RANK to use an alternative for GetNumberSequenceArray() that will accept text that can be converted to numeric values.
Comment 23 Winfried Donkers 2015-03-24 11:18:46 UTC
(In reply to Winfried Donkers from comment #22)
> > However, for functions where Excel and Calc so far produce identical results
> > this illustrates that indeed the number sequence does not include strings
> > converted to numbers, and GetNumberSequenceArray() should not be changed.
> With one exception: RANK.
(and NETWORKDAYS.INTL and WORKDAY.INTL)
Comment 24 Eike Rathke 2015-03-26 14:49:13 UTC
(In reply to Winfried Donkers from comment #22)
> > GetNumberSequenceArray() should not be changed.
> With one exception: RANK.

That's odd. I don't see a reason why that should be treated differently from all other NumberSequence handling.

> http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part2.
> html#__RefHeading__1018894_715980110 led me to believe that text in inline
> arrays ought to be supported.

That says that if inline constant arrays are supported text elements in inline arrays are to be supported. Not that when converting an inline array to a NumberSequence a Text is to be converted to Number.

> But I can of course just make NETWORKDAYS.INTL, WORKDAY.INTL and RANK to use
> an alternative for GetNumberSequenceArray() that will accept text that can
> be converted to numeric values.

Sounds best. With the exception of RANK, I'd not touch that at the moment.
Comment 25 Winfried Donkers 2015-03-26 15:46:05 UTC
(In reply to Eike Rathke from comment #24)
> Sounds best. With the exception of RANK, I'd not touch that at the moment.

I would prefer to add a boolean argument to GetNumberSequenceArray() indicating whether to accept text of not.
I think that the disadvantage (performance penalty) will be minimal and on the plus side there will not be a variant of GetNumberSequenceArray() which is 99% identical. I don't like double code.

I'm ready to start coding right away, but it seems better to mention my intensions before I start.
Any objections?
If so, I'll make a variant of GetNumberSequenceArray().
Comment 26 Winfried Donkers 2015-03-27 06:44:39 UTC
(In reply to Eike Rathke from comment #24)
> > > GetNumberSequenceArray() should not be changed.
> > With one exception: RANK.
> 
> That's odd. I don't see a reason why that should be treated differently from
> all other NumberSequence handling.

I meant with one exception in Excel (RANK), which apparently does handle text conversion to numbers.
Comment 27 Eike Rathke 2015-04-20 15:12:36 UTC
@Winfried:
Can you please fix the document to use ISO 8601 YYYY-MM-DD date strings instead of D-M-YYYY that only works in few locales? Thanks.
Comment 28 Eike Rathke 2015-04-20 15:40:39 UTC
@Winfried:
Could you attach a sample document produced by Excel with the RANK function, where it actually evaluates text in an inline-array as numbers? I tried but didn't succeed. Actually Excel didn't except any inline-array with RANK as second argument, but maybe I messed it up somehow.
Comment 29 Commit Notification 2015-04-20 16:07:53 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=799fd7552f544834ae2d3b77c3ce69e36590b63a

tdf#88547 allow inline date-arrays for Calc array functions

It will be available in 5.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 30 Commit Notification 2015-04-20 16:42:07 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2fcd8c2c40481a95bf0cf59b1dd314d84226226c

take different paths for performance if bAllowText, tdf#88547 follow-up

It will be available in 5.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 31 Commit Notification 2015-04-20 16:42:11 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=b756a907eadb803c8014c7a875bb0b0ee32c50b3

error handling per element and propagate, tdf#88547 follow-up

It will be available in 5.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 32 Commit Notification 2015-04-20 17:02:07 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=46befa93818dc7dedc614a08bf0266b0723f9284

clarify that only text in arrays is to be converted, tdf#88547 follow-up

It will be available in 5.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 33 Commit Notification 2015-04-20 17:02:16 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=887370fe830a148f309b23011f3428d80a53811d

only convert in svMatrix, not in external references, tdf#88547 follow-up

It will be available in 5.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 34 Winfried Donkers 2015-04-21 06:03:21 UTC
(In reply to Eike Rathke from comment #28)
> @Winfried:
> Could you attach a sample document produced by Excel with the RANK function,
> where it actually evaluates text in an inline-array as numbers? I tried but
> didn't succeed. Actually Excel didn't except any inline-array with RANK as
> second argument, but maybe I messed it up somehow.

The Excel document I used, was attachment #114188 [details] saved as xlsx and opened, recalculated and saved again with Excel2010 by Raal. For reasons unclear to me, the formulas for rank have been replaced by numeric values in the xlsx document. I did not notice this and concluded -incorrectly- that Excel handled things differently with the RANK function. So I messed up, not you.
Comment 35 Winfried Donkers 2015-04-21 08:27:25 UTC
Created attachment 114982 [details]
expanded testcase

(In reply to Eike Rathke from comment #27)

Improved attachment.