framework icon indicating copy to clipboard operation
framework copied to clipboard

Fix #1610 unexpected field-error related to a well-formatted value example of a schema's boolean field using frictionless validate

Open amelie-rondot opened this issue 1 year ago • 6 comments

  • fixes #1610

This PR fixes the field-error which occured doing frictionless validate data.csv --schema schema.json with a TableSchema schema containing a BooleanField customised with 'trueValues' or 'falseValues' and 'example' value , for example:

schema.json
----
{
  "$schema": "https://frictionlessdata.io/schemas/table-schema.json",
  "fields": [
    {
      "name": "IsTrue",
      "type": "boolean",
      "trueValues": ["yes"],
      "falseValues": ["no"],
      "example": "yes"
    }
  ]
}
data.csv
----
isTrue
yes
no

This PR add tests cases to ensure that example value is correctly evaluated according to customized 'trueValues' or 'falseValues', as expected.

amelie-rondot avatar Jan 05 '24 15:01 amelie-rondot

Thanks @amelie-rondot! Can you please fix the lining error?

roll avatar Jan 24 '24 15:01 roll

@roll thanks a lot for reviewing! I committed linting fixes.

amelie-rondot avatar Jan 25 '24 16:01 amelie-rondot

Thanks @roll for reviewing! I've committed updates.

amelie-rondot avatar Jan 29 '24 11:01 amelie-rondot

@roll

The change requested have been taken into account and the PR is ready for review !

I noticed some linting errors that seem unrelated to this review, and that I corrected in the other PR (should I have ?). I haven't corrected them in this one as it would mostly lead to merge conflicts. Tell me if you prefer me to do something about it.

pierrecamilleri avatar Apr 23 '24 15:04 pierrecamilleri

I merged the main branch, this PR is ready as well !

pierrecamilleri avatar May 03 '24 08:05 pierrecamilleri

@pdelboca, would it please be possible to prioritize the merges of this bug fix as well as PR #1633 ? Validata currently needs to rely on a fork with these fixes to use frictionless v5, but we wouldn't want this situation to last...

pierrecamilleri avatar Jun 07 '24 07:06 pierrecamilleri

@pierrecamilleri I just merged #1674 . Could you please rebase and then merge when test are passing?

pdelboca avatar Aug 28 '24 09:08 pdelboca