Skip to content

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

Merged
austiezr merged 3 commits into
fivetran:developfrom
sp1thas:support-underscore-in-conditional-value
Jul 6, 2022
Merged

[FEATURE] Support underscore in the condition_value of a row_condition#5393
austiezr merged 3 commits into
fivetran:developfrom
sp1thas:support-underscore-in-conditional-value

Conversation

@sp1thas

@sp1thas sp1thas commented Jun 27, 2022

Copy link
Copy Markdown
Contributor

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

netlify Bot commented Jun 27, 2022

Copy link
Copy Markdown

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

@AFineDayFor AFineDayFor left a comment

Copy link
Copy Markdown
Contributor

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 🙇

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
Copy Markdown
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
Copy Markdown

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

sp1thas commented Jul 4, 2022

Copy link
Copy Markdown
Contributor Author

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 fivetran: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