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

More Error messages needed #103

Open
christian-rli opened this issue May 28, 2024 · 4 comments
Open

More Error messages needed #103

christian-rli opened this issue May 28, 2024 · 4 comments
Assignees

Comments

@christian-rli
Copy link
Collaborator

christian-rli commented May 28, 2024

OMI can benefit from useful error messages.

For example:

  • File doesn't match metadata description
    • column doesn't exist / has different data type / has different name
    • table has more columns than described in metadata
  • JSON seems to be invalid at location X
  • key name does not exist in standards, typo?
  • Warning: no license has been chosen, this field will be necessary when uploading to the OEP

I would recommend creating a list of possible errors first and to then write useful messages and tests for them.

Also, maybe have a look a the license check performed on OEP (implemented by @jh-RLI ) and if it makes sense to put it into OMI or leave it there.

See #99

@christian-rli
Copy link
Collaborator Author

@henhuy I thought up a list of possible error types:

in the metadata:
- wrong structure
- other version than declared
- JSON syntax error (like missing ,}]:'")
- incorrect nesting
- key name not in schema (typo?)
- additional field (this should just show a WARNING. tolerating additional fields has been requested)
- wrong data type in a JSON field
- duplicate key/value
- null values that mess with the structure of the JSON
- missing field (maybe as WARNING? When missing ID or licence)

I have provided a metadata string for each of the errors above in a separate branch. Maybe it's useful to you. I wasn't sure what a minimal metadata string had to include, so maybe they're unnecessarily big.

Checking against data:
- metadata don't match data structure
- missing column
- extra column
- wrong data type in a column (Warning or error?)
- duplicate or null values in ID column

I haven't created samples for these, because I saw that you had a test data table. So I would like use that for any error sample files.

@henhuy
Copy link
Contributor

henhuy commented Jun 17, 2024

Tried to implement metadata checks from above. Metadata files were VERY helpful, thx!
Following findings ("x" means a test is included which checks for given issue):

  • wrong structure
    is covered by either JSON decoder (if structure is no valid JSON format) or JSONschema if structure does not fit
  • other version than declared
    Not yet possible, as only versions >1.5.2 are implemented and there are no differences compared to 1.6.0
  • JSON syntax error (like missing ,}]:'")
    handled by JSON decoder
  • incorrect nesting
    handled by JSON decoder
  • key name not in schema (typo?)
    as we don't have required fields yet, I can not test against it. Typos are out of scope IMO. (wrong_key.json will only throw error due to additional key "titel", but will not know that this is a typo)
  • additional field (this should just show a WARNING. tolerating additional fields has been requested)
    must be allowed in Metadata schema first, otherwise JSONschema will always throw an error
  • wrong data type in a JSON field
    should work due to JSONschema, but not yet tested as types are all "string" or "null"
  • duplicate key/value
    added check to JSON decoder which checks for duplicate keys
  • null values that mess with the structure of the JSON
    handled by JSON decoder
  • missing field (maybe as WARNING? When missing ID or licence)
    this actually raises an error, due to JSONschema

@henhuy
Copy link
Contributor

henhuy commented Jun 17, 2024

Checking against data:

  • metadata don't match data structure
  • missing column
  • extra column
  • wrong data type in a column (Warning or error?)
  • duplicate or null values in ID column

Added tests for all cases.
And: Wrong data type raises an error

@nesnoj
Copy link

nesnoj commented Jun 18, 2024

Great, big thanks for these efforts!
(I guess this will also fix #40)

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

No branches or pull requests

3 participants