search

frictionlessdata / tableschema-js

A JavaScript library for working with Table Schema.
http://frictionlessdata.io/
MIT License
82 stars 27 forks source link

Support collecting all errors by type per row #155

Closed mydu closed 4 years ago

mydu commented 5 years ago

Feature

Collect all errors per row when table.iter/read({forceCast = true}), and group them by type of check with error.type.

roll commented 5 years ago

@mydu Thanks. I'll review as soon as I can

roll commented 5 years ago

@mydu I'm trying to understand the change and it seems good but what I can't fully get:

rows[1].errors[0].errors[0].message

Why is it so nested?

Also is it a breaking change in high-level?

mydu commented 5 years ago

Hi @roll, I tried to collect errors per row and group them by error type (ERROR_FORMAT, ERROR_FOREIGN_KEY, etc), so the structure of my final errors per row became nested, then I have to modify the unit test.

Btw, I also changed to not mutate row when resolve foreignKey if forceCast=true in https://github.com/frictionlessdata/tableschema-js/blob/7e6622a252d389bfa400b9715ea91a20365fd6b5/src/table.js#L166-L183 , otherwise one foreignKey field fails will fail all other foreignKey if I collect errors.

roll commented 5 years ago

Sorry for the slowly going communication... Just got a moment to answer.

Could you please remove this nesting? Users can create this mapping (error type based) by themselves while by default I think it will be less confusing to have it plain. It also will follow the current pattern of having errors with 1 level nesting like multipleError.errors === [...].

Another thing we can consider - following Data Quality Spec (https://github.com/frictionlessdata/data-quality-spec/blob/master/spec.json) regarding error types naming. It's used across our software like goodtables-py, Python table schema libraries etc

paulgirard commented 5 years ago

I might take over this PR as Mengying worked with me and unfortunately had to leave the team. Actually I might try to port the optimization made on the python tableschema in the JS ones. I don't know when I'll have time to do so but I do have that in mind.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.