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

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

Conversation

amelie-rondot
Copy link
Contributor

@amelie-rondot amelie-rondot commented Jan 5, 2024

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.

@roll
Copy link
Member

roll commented Jan 24, 2024

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

@amelie-rondot
Copy link
Contributor Author

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

roll
roll previously requested changes Jan 29, 2024
Copy link
Member

@roll roll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I added a few comments

frictionless/schema/field.py Show resolved Hide resolved
frictionless/schema/field.py Outdated Show resolved Hide resolved
@amelie-rondot
Copy link
Contributor Author

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

@pierrecamilleri
Copy link
Collaborator

@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
Copy link
Collaborator

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

@pierrecamilleri
Copy link
Collaborator

@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...

@pdelboca pdelboca self-requested a review August 20, 2024 06:24
@pdelboca pdelboca dismissed roll’s stale review August 20, 2024 06:25

Changes were implemented.

@pdelboca pdelboca requested review from pierrecamilleri and roll and removed request for pierrecamilleri and roll August 20, 2024 07:11
@pdelboca
Copy link
Contributor

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

@pierrecamilleri pierrecamilleri force-pushed the Fix-1610-solve-field-error-on-boolean-field-with-true-or-false-values-customised branch from 0ea8865 to 42b14fa Compare August 30, 2024 11:08
@pierrecamilleri pierrecamilleri merged commit 8b17d59 into frictionlessdata:main Aug 30, 2024
9 checks passed
@pierrecamilleri pierrecamilleri deleted the Fix-1610-solve-field-error-on-boolean-field-with-true-or-false-values-customised branch August 30, 2024 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected field-error for a boolean "example" with "trueValues" or "falseValues" properties
4 participants