Integrate with loaders.gl 2.2 #1156

manassra · 2020-06-17T23:35:14Z

Most changes in this PR come from #1135 by @heshan0131. The changes this PR brings in addition to those are mostly in src/processors/file-handler.js, where we went through a few iterations of integration with Loaders.gl.

Key New Changes:

Uses the metadata flag to assist with parsing objects
Uses the jsonpaths property to guide the parseInBatches function to what to stream
This bug fix in Loader.gl, allow Kepler to ingest CSVs that have no header row (header is autogenerated as column1, column2, ....)

Next Steps:

Tests for the various dataset types supports (followup PR)
Fix for overflowing upload panel when uploading multiple files simultaneously (followup PR)

heshan0131 · 2020-06-17T23:44:15Z

src/processors/file-handler.js

+  jsonpaths: [
+    '$', // JSON Row array
+    '$.features', // GeoJSON
+    '$.datasets' // KeplerGL JSON


datasets is an array of datasets, which could be less than 10, we can make it be $.datasets.0.data so it always tries to stream the first dataset. And while we save in kepler, we can save the dataset with the longest rows first

Agreed. The reason I left it as $.datasets for now is because it looks like the array indexing in jsonpaths isn't handled properly in loaders.gl.

Yes, I think you can just add a test case in jsonpaths.spec and streaming-json-parser.spec in loaders.gl/json and it should be easy to pin down this little bug (at a guess, some string/number conversion issue perhaps, or openobject, openarray treated differently).

heshan0131 · 2020-06-17T23:45:12Z

src/processors/file-handler.js

@@ -200,9 +178,9 @@ export function processFileData({content, fileCache}) {
    }

    if (format && processor) {
-      console.time('process file content');
+      Console.time('process file content');


this was me testing, it can be removed

ibgreen

Nice work!

Apart from some nits, I think the post processing could merit another look, especially the CSV functions, it feels like there is overlap with loaders and it would be nice to push generic post-processing code upstream into loaders.gl so we can keep the logic here in the app as clean and minimal as possible.

Edit: That would be for another PR.

src/components/common/file-uploader/file-upload.js

src/components/modals/load-data-modal.js

ibgreen · 2020-06-18T01:48:06Z

src/processors/data-processor.js

-  if (!Array.isArray(result) || result.length < 2) {
-    // looks like an empty file, throw error to be catch
-    throw new Error('Read File Failed: CSV is empty');
+export function processCsvData(rawData, header) {


This seems to duplicate the CSVLoader with header detection etc. Can we unify?

Is this unique to CSV data? not other tables? Rename the functions?

Agreed. I think there might be an opportunity to remove this functionality altogether, but it seems to be used for loading sample datasets and other places (if I understand this correctly). @heshan0131 what do you think?

ibgreen · 2020-06-18T01:51:07Z

src/processors/file-handler.js

@@ -157,6 +69,10 @@ export function isRowObject(json) {
  return Array.isArray(json) && isPlainObject(json[0]);
 }

+export function isCsvRows(content) {


This case does not happen any more with the changes in @loaders.gl/csv?

You are right! this isn't needed anymore because a CSV gets transformed to a JSON row array by Loaders.gl. removed.

ibgreen · 2020-06-18T01:51:59Z

src/processors/file-handler.js

+
+    let format;
+    let processor;
+    if (isCsvRows(data)) {


If we need to know if this was originally CSV then we can use the metadata batch instead of sniffing like this. The idea with loaders.gl is that we can add additional streaming parsers do we need to keep track of what type things were originally?

This check isn't actually needed since, as you said, the CSV gets parsed as an array of records (will get captured by the isRowObject(data) instead.

src/reducers/vis-state-updaters.js

Parse JSON using loaders.gl Parse CSV using loaders.gl Let loaders.gl handle file detection, not working though Signed-off-by: Wesam Manassra <[email protected]>

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

Signed-off-by: Wesam Manassra <[email protected]>

… and use jsonpaths Signed-off-by: Wesam Manassra <[email protected]>

Signed-off-by: Wesam Manassra <[email protected]>

Parse JSON using loaders.gl Parse CSV using loaders.gl Let loaders.gl handle file detection, not working though Signed-off-by: Wesam Manassra <[email protected]>

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

Signed-off-by: Wesam Manassra <[email protected]>

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

Signed-off-by: Wesam Manassra <[email protected]>

…to loaders2.2

heshan0131 · 2020-06-18T07:39:30Z

src/processors/file-handler.js


    let format;
    let processor;
-    if (isCsvRows(data)) {
-      format = DATASET_FORMATS.csv;
-      processor = processCsvData;


Let's not delete this. kepler processCsvData does 2 things:

type detection, specifically to find time geo as well as the exact time format yyyy-hh-dd mm:ss

basic data cleaning. lots of times people have string values like N/A Null N/ mixed up with numbers. kepler finds them and fill it in with null so type detect won't fail.

For case1, there is a fall back in reducer, if format if time is note detect, keler will call type detection again. Not sure loaders.gl will handle case 2

Right now loaders doesn't do this, but it will have to when we switch the loaders to return columnar data batches (typed arrays) and arrow batches.

I think we should look at moving the data cleaning out as a second step.

To add to Ib's point, once the dataset is processed via loaders.gl, the output format for CSV files and JSON Row objects becomes the same (an array of records), so the isCsvRows function doesn't get evaluated. Any ideas on how to tackle this?

Signed-off-by: Shan He <[email protected]>

add probe.gl browser drive test test row object and kepler map Signed-off-by: Shan He <[email protected]>

heshan0131 · 2021-03-20T21:12:59Z

#76

manassra requested review from macrigiuseppe, heshan0131 and ibgreen June 17, 2020 23:35

heshan0131 reviewed Jun 17, 2020

View reviewed changes

ibgreen changed the title ~~[WIP] Integrate with Loaders 2.2~~ Integrate with loaders.gl 2.2 Jun 18, 2020

ibgreen approved these changes Jun 18, 2020

View reviewed changes

gnandretta and others added 13 commits June 17, 2020 22:35

Install loaders.gl packages

adbef9a

Parse JSON using loaders.gl Parse CSV using loaders.gl Let loaders.gl handle file detection, not working though Signed-off-by: Wesam Manassra <[email protected]>

set batchSize to loaders

5cc4f6f

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

add run:loaders-src script

ffacf14

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

rendering file upload progress

9fbdada

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

rendeirng progress

d45c75d

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

cleanup load file updater logic

9bedfac

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

cleanup

c25e1c6

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

metadata

a589550

Signed-off-by: Wesam Manassra <[email protected]>

Clean and organize file-handler.js

bd52e83

Signed-off-by: Wesam Manassra <[email protected]>

Clean up comments

f3b4393

Signed-off-by: Wesam Manassra <[email protected]>

Update loading logic to use the metadata flag, handle no-header csvs,…

44d3d2a

… and use jsonpaths Signed-off-by: Wesam Manassra <[email protected]>

Address cosmetic feedback

0fc4c87

Signed-off-by: Wesam Manassra <[email protected]>

Remove csv processing from processFileData

b142795

Signed-off-by: Wesam Manassra <[email protected]>

manassra force-pushed the loaders2.2 branch from 304ff23 to b142795 Compare June 18, 2020 05:36

manassra and others added 9 commits June 18, 2020 00:10

Fix/Add types

6edde92

Install loaders.gl packages

285b4ad

Parse JSON using loaders.gl Parse CSV using loaders.gl Let loaders.gl handle file detection, not working though Signed-off-by: Wesam Manassra <[email protected]>

set batchSize to loaders

8963235

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

add run:loaders-src script

ded26e2

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

rendering file upload progress

9112bd3

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

rendeirng progress

7aea7ad

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

cleanup load file updater logic

c11f611

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

cleanup

94dbd6b

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

metadata

b5f0206

Signed-off-by: Wesam Manassra <[email protected]>

heshan0131 and others added 8 commits June 18, 2020 00:26

cleanup load file updater logic

f951939

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

cleanup

4aba2c4

Signed-off-by: Shan He <[email protected]> Signed-off-by: Wesam Manassra <[email protected]>

metadata

eb496bd

Signed-off-by: Wesam Manassra <[email protected]>

Clean and organize file-handler.js

6056d44

Signed-off-by: Wesam Manassra <[email protected]>

Address cosmetic feedback

25fef2d

Signed-off-by: Wesam Manassra <[email protected]>

Fix/Add types

27fa11f

Signed-off-by: Wesam Manassra <[email protected]>

Remove duplicate declarations

1eab053

Merge branch 'loaders2.2' of https://github.com/keplergl/kepler.gl in…

4aaa82c

…to loaders2.2

heshan0131 reviewed Jun 18, 2020

View reviewed changes

manassra and others added 9 commits June 18, 2020 10:39

Fix typescript errors

a802642

fix unit tests

a7c4d93

[bug] fix the action name for NEXT_FILE_BATCH

62bba6e

Fix bug in unit tests for file uploader

688d237

return tap-spec

a9c2821

Increase e2e timeouts

315e32b

Remove E2E tests from CI

f2662e7

add test

5839d17

Signed-off-by: Shan He <[email protected]>

add file handler tests

678eb99

Signed-off-by: Shan He <[email protected]>

manassra mentioned this pull request Jun 23, 2020

Bump deck.gl and luma.gl to v8.2 #1166

Merged

manassra and others added 5 commits June 30, 2020 14:28

merge with master

4b90ab0

remove console.log in file-handler-test

1b7d8c8

fix lint errors

563cc83

fix isKeplerGLMap Implementation

d0733a4

add loaders.gl tests

b38e4a3

add probe.gl browser drive test test row object and kepler map Signed-off-by: Shan He <[email protected]>

heshan0131 merged commit 01484c6 into master Jul 1, 2020

delete-merged-branch bot deleted the loaders2.2 branch July 1, 2020 08:00

heshan0131 mentioned this pull request Jul 1, 2020

[WIP] loaders.gl integration #1135

Closed

heshan0131 mentioned this pull request Mar 20, 2021

Increase upload limit #76

Closed

heshan0131 mentioned this pull request Mar 20, 2021

Increase upload dataset limit #410

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate with loaders.gl 2.2 #1156

Integrate with loaders.gl 2.2 #1156

manassra commented Jun 17, 2020 •

edited

Loading

heshan0131 Jun 17, 2020

manassra Jun 18, 2020

ibgreen Jun 18, 2020

heshan0131 Jun 17, 2020

ibgreen left a comment •

edited

Loading

ibgreen Jun 18, 2020

manassra Jun 18, 2020

ibgreen Jun 18, 2020

manassra Jun 18, 2020

ibgreen Jun 18, 2020

manassra Jun 18, 2020

heshan0131 Jun 18, 2020

ibgreen Jun 18, 2020

manassra Jun 18, 2020

heshan0131 commented Mar 20, 2021

Integrate with loaders.gl 2.2 #1156

Integrate with loaders.gl 2.2 #1156

Conversation

manassra commented Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibgreen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heshan0131 commented Mar 20, 2021

manassra commented Jun 17, 2020 •

edited

Loading

ibgreen left a comment •

edited

Loading