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.

Sign up for GitHub

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

Jump to bottom

New NumberParser for Table parsing #11499

Merged

mergify merged 35 commits into develop from wip/jd/number-parser

Nov 13, 2024

Member

jdunkerley commented Nov 6, 2024 •

edited

Loading

Pull Request Description

Replaces the Regex based number parser with a new parser which works out the same by working out each part as it sees and example of it.

Close #7398 - performance of reading the large CSV now about 2s (down from 15-20s).

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.
If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
or the Snowflake database integration, a run of the Extra Tests has been scheduled.
- If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

jdunkerley mentioned this pull request

Improve performance of number parsing #11228

Open

jdunkerley added the CI: No changelog needed label

GregoryTravis approved these changes

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/FormatDetectingNumberParser.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Table/0.0.0-dev/src/Data_Formatter.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/FormatDetectingNumberParser.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NegativeSign.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/table/src/main/java/org/enso/table/parsing/NumberParser.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/table/src/main/java/org/enso/table/parsing/NumberParser.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/table/src/main/java/org/enso/table/parsing/NumberParser.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/table/src/main/java/org/enso/table/parsing/NumberParser.java Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/FormatDetectingNumberParser.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/FormatDetectingNumberParser.java Outdated Show resolved Hide resolved

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/Separators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/base/src/main/java/org/enso/base/parser/NumberWithSeparators.java Outdated Show resolved Hide resolved

jdunkerley added 15 commits

November 12, 2024 18:28


          Imply isInteger from the fact we have an Integer Type.

dda577f


          Comments on NumberWithSeparators.

6f967df


          Replace Stream with switch statements.

cdccdb8


          idx -> startIdx

219d094


          javafmt

64041b3


          More PR bits.

d2be5d9


          PR work and starting on Scientific Notation.

afe78a1


          PR comments and bugs.

ae9f47f


          Fix a couple of tests.

3e0e1c0


          Java format.

71be20d


          More PR comments.

4d30030


          Block whitespace immediately following signs.

1ebbaee


          Remove the US special bit.

af6ce6f


          Fix some issues.

987b882

To fix exponentialNotation and validation of thousands.


          Java format.

577ee67

jdunkerley force-pushed the wip/jd/number-parser branch from f7d4754 to 577ee67 Compare

November 12, 2024 18:30

jdunkerley added 11 commits

November 12, 2024 20:13


          Fix broken if statement.

beed79b


          Validate final thousand if no decimal.

eb9ab1a


          Fix for matching . and ,.

79bcb69


          Fix for -0.

af6e010

Don't convert Long to Double let builder do that.


          Fix for a US issue

67aec16


          Fix for a US issue

89b182a


          Functioning exponential notation.

f5b6913


          Java Format.

cf8fb90


          Reduce the repeat parsing.

f408678


          Fixes to edge cases broken in the patching.

f667d3a


          Java Format.

94ec4c6

jdunkerley added the CI: Ready to merge label

mergify bot merged commit 6b54465 into develop

37 checks passed

mergify bot deleted the wip/jd/number-parser branch

November 13, 2024 19:08

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

GregoryTravis GregoryTravis approved these changes

radeusgd radeusgd approved these changes

AdRiley Awaiting requested review from AdRiley AdRiley is a code owner

marthasharkey Awaiting requested review from marthasharkey marthasharkey is a code owner

Labels

CI: No changelog needed CI: Ready to merge