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

[FEATURE] DataProfilerColumnDomainBuilder #7920

Merged

Conversation

micdavis
Copy link
Contributor

@micdavis micdavis commented May 17, 2023

Changes proposed in this pull request:

  • Updated DataProfilerColumnDomainBuilder class to remove profile_path instance variable.

  • Updated DataProfilerColumnDomainBuilder's _get_domains() function to take in profile_path, metric, and metric_values as a part of variables

  • Updated DataProfilerTableColumnList to filter columns from the profile object by checking if the metric of the column is in metric_values

  • Updated test_data_profiler_column_domain_builder and test_data_profiler_structured_data_assistant to match updated DataProfilerColumnDomainBuilder functionality

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

@netlify
Copy link

netlify bot commented May 17, 2023

Deploy Preview for niobium-lead-7998 ready!

Name Link
🔨 Latest commit 08c4d76
🔍 Latest deploy log https://app.netlify.com/sites/niobium-lead-7998/deploys/646649b7cb87ba0008560126
😎 Deploy Preview https://deploy-preview-7920--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.

@micdavis
Copy link
Contributor Author

This PR fixes issues with #7770. Had to make a new PR because I don't have write access to @alexjeilert remote. #7770 can be closed out. If there are no questions, comments, or concerns with this @alexsherstinsky please enable auto-merge.

@alexsherstinsky alexsherstinsky enabled auto-merge (squash) May 18, 2023 15:39
Copy link
Contributor

@alexsherstinsky alexsherstinsky left a comment

Choose a reason for hiding this comment

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

LGTM!

@alexsherstinsky alexsherstinsky merged commit 0aa1209 into great-expectations:develop May 18, 2023
29 checks passed
Shinnnyshinshin added a commit that referenced this pull request May 18, 2023
* develop:
  [MAINTENANCE] Update `teams.yml` (#7934)
  [RELEASE] 0.16.13 (#7933)
  [MAINTENANCE] Fixing pytest: renderer assertion (#7928)
  [FEATURE] Add "batch.columns()" convenience method to Fluent DataAsset implementation. (#7926)
  [FEATURE] DataProfilerColumnDomainBuilder (#7920)
  [MAINTENANCE] Fixes Glue tests on vanilla pytest (#7925)
  [DOCS] CLI Clean-up (#7904)
  [FEATURE]: DataProfilerStructuredDataAssistant Float Rule (#7842)
  [BUGFIX] Fix remaining FDS config substitution issues (#7917)
  [FEATURE] Implementing Python code snippets under test for "https://docs.greatexpectations.io/docs/guides/connecting_to_your_data/fluent/filesystem/how_to_connect_to_one_or_more_files_using_pandas" (#7922)
  [DOCS] How to Edit Existing `ExpectationSuite` (#7859)
  [MAINTENANCE] Add `docs/*.py` to `GXChanged` for linting  (#7924)
  [MAINTENANCE] Use with open file to reduce open files within tests (#7906)
  [MAINTENANCE] Removing engine-specific tests that assert generic behavior (#7918)
  [FEATURE] Implementing Python code snippets under test for "https://docs.greatexpectations.io/docs/guides/connecting_to_your_data/fluent/data_assets/how_to_organize_batches_in_a_sql_based_data_asset" (#7909)
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.

None yet

4 participants