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

added --display-cache to jupyter magic #1225

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Conversation

zilto
Copy link
Collaborator

@zilto zilto commented Nov 12, 2024

Follows #1183. Also it switched the default of --execute to not show the execution results. The flag --hide_results was changed to --show_results in a backwards compatible way. The example notebook was updated accordingly.

@zilto zilto added enhancement New feature or request plugin Relates to a Hamilton plugin (adapters, materializers, etc.) labels Nov 12, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 138958a in 45 seconds

More details
  • Looked at 92 lines of code in 1 files
  • Skipped 1 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. hamilton/plugins/jupyter_magic.py:189
  • Draft comment:
    Consider rephrasing the deprecation warning for --hide_results to make it clearer that --show_results is the new way to display results.
"DeprecationWarning: `--hide_results` is deprecated. Results are now hidden by default when using `--execute`. Use `--show_results` to display them."
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The deprecation warning for --hide_results is clear, but the message could be slightly improved for clarity.
2. hamilton/plugins/jupyter_magic.py:238
  • Draft comment:
    Clarify that --display_cache displays the cache visualization, not just results.
help="After execution, display the cache visualization using `dr.cache.view_run()`.",
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The help message for --display_cache should clarify that it displays the cache visualization, not just results.
3. hamilton/plugins/jupyter_magic.py:270
  • Draft comment:
    Update the help message for --show_results to reflect that results are hidden by default.
help="Show node values in the output cell after each node is executed. Results are hidden by default.",
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The help message for --show_results should be updated to reflect that results are hidden by default.
4. hamilton/plugins/jupyter_magic.py:317
  • Draft comment:
    Ensure consistency in error messages by using print instead of return for KeyError.
print(f"KeyError: Received `--{name} {value}` but variable not found.")
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The error message for KeyError should be consistent with other error messages in the file.

Workflow ID: wflow_fuBkVksRZ5dZ69Rv


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@skrawcz skrawcz merged commit 9309981 into main Nov 13, 2024
24 checks passed
@skrawcz skrawcz deleted the feat/notebook-display-cache branch November 13, 2024 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plugin Relates to a Hamilton plugin (adapters, materializers, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants