-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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] OnboardingDataAssistantResult plotting feature parity with VolumeDataAssistantResult #5145
[FEATURE] OnboardingDataAssistantResult plotting feature parity with VolumeDataAssistantResult #5145
Conversation
✅ Deploy Preview for niobium-lead-7998 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
…ta-assistant-table-plots
…ta-assistant-table-plots
…ta-assistant-table-plots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I'd like to see if we can avoid those string to string maps at all. Let me know your thoughts.
great_expectations/rule_based_profiler/types/data_assistant_result/data_assistant_result.py
Show resolved
Hide resolved
great_expectations/rule_based_profiler/types/data_assistant_result/data_assistant_result.py
Show resolved
Hide resolved
great_expectations/rule_based_profiler/types/data_assistant_result/data_assistant_result.py
Show resolved
Hide resolved
...ctations/rule_based_profiler/types/data_assistant_result/onboarding_data_assistant_result.py
Show resolved
Hide resolved
tests/rule_based_profiler/data_assistant/test_onboarding_data_assistant.py
Show resolved
Hide resolved
): | ||
context: DataContext = bobby_columnar_table_multi_batch_deterministic_data_context | ||
|
||
theme = {"font": "Comic Sans MS"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤣
tests/rule_based_profiler/data_assistant/test_onboarding_data_assistant.py
Show resolved
Hide resolved
…ta-assistant-table-plots
...expectations/rule_based_profiler/types/data_assistant_result/volume_data_assistant_result.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NathanFarmer just requesting the type hints (OK otherwise) -- thank you!
…able-plots' of github.com:great-expectations/great_expectations into feature/GREAT-761/GREAT-847/onboarding-data-assistant-table-plots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes proposed in this pull request:
VolumeDataAssistantResult
in preparation forOnboardingDataAssistantResult
plottingDataAssistantResult
allowing for thin wrapper at child class levelDefinition of Done