Skip to content
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

Native Excel dates that happen to be a round number getting mistyped #1416

Closed
orcinus opened this issue Mar 12, 2020 · 14 comments · Fixed by #3121
Closed

Native Excel dates that happen to be a round number getting mistyped #1416

orcinus opened this issue Mar 12, 2020 · 14 comments · Fixed by #3121

Comments

@orcinus
Copy link

orcinus commented Mar 12, 2020

This is:

- [X] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

All native Excel dates parsed from an Excel spreadsheet should be cast into a float.

What is the current behavior?

Native Excel dates that just so happen to be a round number end up cast into an integer, while those that are not round are correctly cast as float.

What are the steps to reproduce?

  1. create a new spreadsheet
  2. write "08/03/2020 00:00:00" into first cell
  3. write "07/03/2020 07:00:00" into second cell
  4. parse the spreadsheet
  5. apply getCalculatedValue() on first and second cell
  6. first cell's value gets typed as an integer
  7. second cell's value gets typed as a float

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:

Not possible, the problem occurs when reading/parsing a file.

Which versions of PhpSpreadsheet and PHP are affected?

It started after 1.8.2. Sadly, i cannot be more precise than that. I was working on a stale branch with 1.8.2 when i got a ticket about an issue that ended up being caused by this bug in production. After hunting around for an hour trying to figure out what's going on, as i couldn't repro the case on my (stale) branch, i realized it might not be our code, updated my PhpSpreadsheet to 1.11.0 and the problem showed up.

We're running PHP 7.1.33

@stale
Copy link

stale bot commented May 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label May 13, 2020
@orcinus
Copy link
Author

orcinus commented May 17, 2020

Bump.

@stale stale bot removed the stale label May 17, 2020
@stale
Copy link

stale bot commented Jul 18, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Jul 18, 2020
@orcinus
Copy link
Author

orcinus commented Jul 22, 2020

Bump again.

@stale stale bot removed the stale label Jul 22, 2020
@stale
Copy link

stale bot commented Sep 20, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Sep 20, 2020
@orcinus
Copy link
Author

orcinus commented Sep 20, 2020

So, how's the weather up there?

@stale stale bot removed the stale label Sep 20, 2020
@pittyplatsch
Copy link

Hi there! On my personal #hacktoberfest quest I came across this issue. Unfortunately couln't reproduce. Can you maybe provide the file you're trying to read?

@alexgunkel
Copy link
Contributor

I could reproduce the behavior. Depending on your file you get a numeric value that either contains an integer or a floating point number. Therefor, it's interpreted as an integer if the string representation contains a dot, as float otherwise. This is expected behavior as far as I can see.

I recommend casting it after calling getCalculatedValue when you know that you got a date and time and you need a float.

@orcinus
Copy link
Author

orcinus commented Oct 24, 2020

Exactly this. But, if it's a datetime field, it should always be (re)cast into a float, regardless of whether the representation ended up being an integer (by accident) or a float. Because an excel datetime is, by definition, internally a float.

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Dec 25, 2020
@orcinus
Copy link
Author

orcinus commented Dec 25, 2020

🎄🎄🎄Merry Christmas, stale-bot!🎄🎄🎄

Ugh. Since there are no takers, i guess i'll try and whip up a PR myself one of these days.

@stale stale bot removed the stale label Dec 25, 2020
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Oct 13, 2022
Fix PHPOffice#1416. I do not entirely understand the use case for this old issue, but resolving it seems straightforward. Issue complains that user-entered date/time fields may be interpreted as either float or int when PhpSpreadsheet reads them. Issue suggests getCalculatedValue treat all date/time fields as float; that seems like a breaking change. However, adding an option to permit it seems okay. That option might be implemented as either a property of Calculation, or a static property of Cell. Since the changed logic is found in Cell (and Shared/Date), I opted for the latter.

In Cell, the property `$parent` is incorrectly described in doc block as `Cells`, and should be `?Cells`. This change eliminates some Phpstan and Scrutinizer problems, and should allow the elimination of some try/catch blocks - I have not done an exhaustive search for those.

Calls to `isDateTime` could have affected activeSheet and selectedCells; they no longer can. Optional parameters are added to it and the functions it calls to accommodate the new functionality; the defaults for the new parameters will, of course, return the same result as the earlier versions of the functions would have returned.
oleibman added a commit that referenced this issue Oct 19, 2022
)

* Permit Date/Time Entered on Spreadsheet to be Calculated as Float

Fix #1416. I do not entirely understand the use case for this old issue, but resolving it seems straightforward. Issue complains that user-entered date/time fields may be interpreted as either float or int when PhpSpreadsheet reads them. Issue suggests getCalculatedValue treat all date/time fields as float; that seems like a breaking change. However, adding an option to permit it seems okay. That option might be implemented as either a property of Calculation, or a static property of Cell. Since the changed logic is found in Cell (and Shared/Date), I opted for the latter.

In Cell, the property `$parent` is incorrectly described in doc block as `Cells`, and should be `?Cells`. This change eliminates some Phpstan and Scrutinizer problems, and should allow the elimination of some try/catch blocks - I have not done an exhaustive search for those.

Calls to `isDateTime` could have affected activeSheet and selectedCells; they no longer can. Optional parameters are added to it and the functions it calls to accommodate the new functionality; the defaults for the new parameters will, of course, return the same result as the earlier versions of the functions would have returned.

* Scrutinizer - Self-inflicted

Tests used constant which I deprecated.
@orcinus
Copy link
Author

orcinus commented Oct 19, 2022

HUZZAH!
REJOICE!

Thank you :)

@oleibman
Copy link
Collaborator

You're welcome. Please note that you need to add an extra function call to activate the new behavior; it would otherwise have been a breaking change.

@orcinus
Copy link
Author

orcinus commented Oct 19, 2022

Noted.
I hacked a fix in into our internal fork in a similar way for similar reasons, actually.

MarkBaker added a commit that referenced this issue Dec 21, 2022
### Added

- Extended flag options for the Reader `load()` and Writer `save()` methods
- Apply Row/Column limits (1048576 and XFD) in ReferenceHelper [PR #3213](#3213)
- Allow the creation of In-Memory Drawings from a string of binary image data, or from a stream. [PR #3157](#3157)
- Xlsx Reader support for Pivot Tables [PR #2829](#2829)
- Permit Date/Time Entered on Spreadsheet to be calculated as Float [Issue #1416](#1416) [PR #3121](#3121)

### Changed

- Nothing

### Deprecated

- Direct update of Calculation::suppressFormulaErrors is replaced with setter.
- Font public static variable defaultColumnWidths replaced with constant DEFAULT_COLUMN_WIDTHS.
- ExcelError public static variable errorCodes replaced with constant ERROR_CODES.
- NumberFormat constant FORMAT_DATE_YYYYMMDD2 replaced with existing identical FORMAT_DATE_YYYYMMDD.

### Removed

- Nothing

### Fixed

- Fixed handling for `_xlws` prefixed functions from Office365 [Issue #3245](#3245) [PR #3247](#3247)
- Conditionals formatting rules applied to rows/columns are removed [Issue #3184](#3184) [PR #3213](#3213)
- Treat strings containing currency or accounting values as floats in Calculation Engine operations [Issue #3165](#3165) [PR #3189](#3189)
- Treat strings containing percentage values as floats in Calculation Engine operations [Issue #3155](#3155) [PR #3156](#3156) and [PR #3164](#3164)
- Xlsx Reader Accept Palette of Fewer than 64 Colors [Issue #3093](#3093) [PR #3096](#3096)
- Use Locale-Independent Float Conversion for Xlsx Writer Custom Property [Issue #3095](#3095) [PR #3099](#3099)
- Allow setting AutoFilter range on a single cell or row [Issue #3102](#3102) [PR #3111](#3111)
- Xlsx Reader External Data Validations Flag Missing [Issue #2677](#2677) [PR #3078](#3078)
- Reduces extra memory usage on `__destruct()` calls [PR #3092](#3092)
- Additional properties for Trendlines [Issue #3011](#3011) [PR #3028](#3028)
- Calculation suppressFormulaErrors fix [Issue #1531](#1531) [PR #3092](#3092)
- Permit Date/Time Entered on Spreadsheet to be Calculated as Float [Issue #1416](#1416) [PR #3121](#3121)
- Incorrect Handling of Data Validation Formula Containing Ampersand [Issue #3145](#3145) [PR #3146](#3146)
- Xlsx Namespace Handling of Drawings, RowAndColumnAttributes, MergeCells [Issue #3138](#3138) [PR #3136](#3137)
- Generation3 Copy With Image in Footer [Issue #3126](#3126) [PR #3140](#3140)
- MATCH Function Problems with Int/Float Compare and Wildcards [Issue #3141](#3141) [PR #3142](#3142)
- Fix ODS Read Filter on number-columns-repeated cell [Issue #3148](#3148) [PR #3149](#3149)
- Problems Formatting Very Small and Very Large Numbers [Issue #3128](#3128) [PR #3152](#3152)
- XlsxWrite preserve line styles for y-axis, not just x-axis [PR #3163](#3163)
- Xlsx Namespace Handling of Drawings, RowAndColumnAttributes, MergeCells [Issue #3138](#3138) [PR #3137](#3137)
- More Detail for Cyclic Error Messages [Issue #3169](#3169) [PR #3170](#3170)
- Improved Documentation for Deprecations - many PRs [Issue #3162](#3162)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants