Skip to content

[FEATURE] Support underscore in the condition_value of a row_condition #5393

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

Merged
merged 3 commits into from
Jul 6, 2022
Merged

[FEATURE] Support underscore in the condition_value of a row_condition #5393

merged 3 commits into from
Jul 6, 2022

Conversation

sp1thas
Copy link
Contributor

@sp1thas sp1thas commented Jun 27, 2022

Changes proposed in this pull request:

  • Allow underscore charachter (_) in a condition_value

This PR aims to allow the user to add an underscore in the condition value of a row condition. For instance, after merging this PR, the following expectation is going to work instead of raising: ConditionParserError: unable to parse condition: col("foo")=="a_b"

{
  "data_asset_type": null,
  "expectation_suite_name": "foo",
  "expectations": [
    {
      "expectation_type": "expect_column_values_to_be_between",
      "kwargs": {
        "column": "foo",
        "min_value": 1,
        "max_value": 2,
        "condition_parser": "great_expectations__experimental__",
        "row_condition": "col(\"foo\")==\"a_b\""
      }
    }
  ]
}

Definition of Done

Please delete options that are not relevant.

  • My code follows the Great Expectations style guide
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added unit tests where applicable and made sure that new and existing tests are passing.
  • I have run any local integration tests and made sure that nothing is broken.

Thank you for submitting!

@netlify
Copy link

netlify bot commented Jun 27, 2022

Deploy Preview for niobium-lead-7998 canceled.

Name Link
🔨 Latest commit 6e7cb2a
🔍 Latest deploy log https://app.netlify.com/sites/niobium-lead-7998/deploys/62ba0ab46c5fd700084ebbdf

@sp1thas sp1thas changed the title Support underscore in condition_value of a row_condition Support underscore in the condition_value of a row_condition Jun 27, 2022
Copy link
Contributor

@AFineDayFor AFineDayFor left a comment

Choose a reason for hiding this comment

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

Thank you very much for identifying this and addressing it 🙇

@@ -42,9 +42,9 @@ def _set_notnull(s, l, t) -> None:
eq = Literal("==")
ops = (gt ^ lt ^ ge ^ le ^ eq).setResultsName("op")
fnumber = Regex(r"[+-]?\d+(?:\.\d*)?(?:[eE][+-]?\d+)?").setResultsName("fnumber")
condition_value = Suppress('"') + Word(f"{alphanums}.").setResultsName(
condition_value = Suppress('"') + Word(f"{alphanums}._").setResultsName(
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

@sp1thas sp1thas changed the title Support underscore in the condition_value of a row_condition [FEATURE] Support underscore in the condition_value of a row_condition Jun 30, 2022
@andrewm4894
Copy link

What would be involved in getting this merged?

Maybe @sp1thas would need to rebase to develop?

I have some long format tables and being able to allow "_" where each row is a metric name of some sort will be really useful.

@sp1thas
Copy link
Contributor Author

sp1thas commented Jul 4, 2022

I was about to ask the same @andrewm4894 . Based on #5387 (which is quite recent), merging commis into develop from <GITHUB-USER>:<FEATURE-BRANCH> seems to be the case. I guess another maintainer should review this PR too. @AFineDayFor do you have any clue?

@austiezr austiezr enabled auto-merge (squash) July 6, 2022 15:22
@austiezr austiezr merged commit 5c2f0a9 into great-expectations:develop Jul 6, 2022
Shinnnyshinshin pushed a commit that referenced this pull request Jul 7, 2022
…ps://github.com/great-expectations/great_expectations into feature/GREAT-1009/remaining-migration-for-init

* 'feature/GREAT-1009/remaining-migration-for-init' of https://github.com/great-expectations/great_expectations:
  [MAINTENANCE] Update cli with new default simple checkpoint name (#5450)
  [MAINTENANCE] Add unit and integration tests for Splitting on Divided Integer (#5449)
  support underscore in condition value (#5393)
  Remove checkpoint from top level of schema since it is captured in `meta` (#5445)
  [MAINTENANCE] Add checkpoint name to validation results (#5442)
Shinnnyshinshin pushed a commit that referenced this pull request Jul 7, 2022
…eature/GREAT-954/data-assistant-logic-migration

* feature/GREAT-1009/remaining-migration-for-init:
  clean up
  [MAINTENANCE] Update cli with new default simple checkpoint name (#5450)
  [MAINTENANCE] Add unit and integration tests for Splitting on Divided Integer (#5449)
  support underscore in condition value (#5393)
  Remove checkpoint from top level of schema since it is captured in `meta` (#5445)
  [MAINTENANCE] Add checkpoint name to validation results (#5442)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants