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

[BUGFIX] Contrib Expectation tracebacks #6471

Merged
merged 5 commits into from
Dec 1, 2022

Conversation

kenwade4
Copy link
Contributor

@kenwade4 kenwade4 commented Dec 1, 2022

The build_galllery pipeline has not successfully run since some contrib Expectations starting throwing tracebacks during the build process. This fixes them.

Changes proposed in this pull request:

  • Specify the file encoding for the MIC csv file for pd.read_csv
  • Fix TypeError: 'ValidationDependencies' object is not subscriptable
  • Fix ImportError: cannot import name '...' from 'great_expectations.expectations.metrics'

@github-actions github-actions bot added the devrel This item is being addressed by the Developer Relations Team label Dec 1, 2022
@netlify
Copy link

netlify bot commented Dec 1, 2022

Deploy Preview for niobium-lead-7998 ready!

Name Link
🔨 Latest commit 0f822c8
🔍 Latest deploy log https://app.netlify.com/sites/niobium-lead-7998/deploys/638907377307610009bdd344
😎 Deploy Preview https://deploy-preview-6471--niobium-lead-7998.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@@ -33,7 +33,7 @@ def is_valid_mic(mic_code: str, df) -> bool:
class ColumnValuesToBeValidMic(ColumnMapMetricProvider):

url = "https://www.iso20022.org/sites/default/files/ISO10383_MIC/ISO10383_MIC.csv"
df = pd.read_csv(url)
df = pd.read_csv(url, encoding="cp1250")
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do?

Copy link
Contributor

Choose a reason for hiding this comment

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

From discussion : this is the encoding of the specific CSV we are using in the test (was failing with unicode)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the actual encoding of the file at that URL

Copy link
Contributor

@Shinnnyshinshin Shinnnyshinshin left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @kenwade4

@kenwade4 kenwade4 enabled auto-merge (squash) December 1, 2022 19:58
@kenwade4 kenwade4 merged commit 9e20696 into develop Dec 1, 2022
@kenwade4 kenwade4 deleted the bugfix/contrib-expectation-tracebacks branch December 1, 2022 20:01
@ghost
Copy link

ghost commented Dec 1, 2022

👇 Click on the image for a new way to code review
  • Make big changes easier — review code in small groups of related files

  • Know where to start — see the whole change at a glance

  • Take a code tour — explore the change with an interactive tour

  • Make comments and review — all fully sync’ed with github

    Try it now!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

Shinnnyshinshin pushed a commit that referenced this pull request Dec 5, 2022
* develop: (63 commits)
  [FEATURE] Support to include ID/PK in validation result for each row - SQL (#6448)
  [BUGFIX] Support slack channel name with webhook also (#6481)
  Query the database for datetime column splitter defaults (#6482)
  [MAINTENANCE] Move "Domain" to "great_expectations/core" to avoid circular imports; also add MetricConfiguration tests; and other clean up. (#6484)
  [MAINTENANCE] Reformat core expectation docstrings (#6423)
  [MAINTENANCE] Staging for build gallery (#6480)
  [MAINTENANCE] Move zep method from datasource to data asset. (#6477)
  [MAINTENANCE] Minor cleanup for better code readability (#6478)
  [MAINTENANCE] Misc updates to PR template (#6479)
  [CONTRIB] Add uniqueness expectation (#6473)
  [RELEASE] 0.15.36 (#6476)
  Add pretty representations for zep pydantic models (#6472)
  [BUGFIX] Contrib Expectation tracebacks (#6471)
  [BUGFIX] Add additional error checking to `ExpectationAnonymizer` (#6467)
  Add docstring for context.sources.add_postgres (#6459)
  [MAINTENANCE] fixing type hints in metrics utils module (#6469)
  [MAINTENANCE] Moving tutorials to great-expectations repo (#6464)
  [BUGFIX] Patch issue with call to `ExpectationAnonymizer` to ensure `DataContext` init events are captured (#6458)
  [BUGFIX] Support Table and Column Names Case Non-Sensitivity Relationship Between Snowflake, Oracle, DB2, etc. DBMSs (Upper Case) and SQLAlchemy (Lower Case) Representations (#6450)
  Add sorters to zep postgres datasource. (#6456)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devrel This item is being addressed by the Developer Relations Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants