flytekit icon indicating copy to clipboard operation
flytekit copied to clipboard

Expose all settings in ydata-profiling ProfileReport

Open eapolinario opened this issue 1 year ago • 3 comments

Why are the changes needed?

ydata-profiling ProfileReport has a rich set of settings that should be exposed in flyte decks.

What changes were proposed in this pull request?

Expose kwargs passed directly to the constructor of ProfileReport.

I set the minimal version of ydata-profiling to 2.4.0 to support minimal-mode.

How was this patch tested?

Setup process

Screenshots

Check all the applicable boxes

  • [ ] I updated the documentation accordingly.
  • [ ] All new and existing tests passed.
  • [ ] All commits are signed-off.

Related PRs

Related to https://github.com/flyteorg/flyte/issues/5993

Docs link

Summary by Bito

This PR enhances ydata-profiling integration in flytekit-deck-standard by exposing all ProfileReport constructor settings. The FrameProfilingRenderer now supports custom configuration via kwargs while maintaining backward compatibility. The minimum ydata-profiling version is set to 2.4.0 to support features like minimal-mode.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 2

eapolinario avatar Nov 12 '24 02:11 eapolinario

/review

eapolinario avatar Dec 27 '24 19:12 eapolinario

Code Review Agent Run #3d8b84

Actionable Suggestions - 1
  • plugins/flytekit-deck-standard/tests/test_renderer.py - 1
    • Consider removing duplicate test case · Line 44-45
Additional Suggestions - 1
  • plugins/flytekit-deck-standard/flytekitplugins/deck/renderer.py - 1
    • Consider removing redundant kwargs check · Line 64-65
Review Details
  • Files reviewed - 4 · Commit Range: 3793e84..3793e84
    • plugins/flytekit-deck-standard/dev-requirements.in
    • plugins/flytekit-deck-standard/flytekitplugins/deck/renderer.py
    • plugins/flytekit-deck-standard/setup.py
    • plugins/flytekit-deck-standard/tests/test_renderer.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

eapolinario avatar Dec 27 '24 19:12 eapolinario

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Feature Improvement - Enhanced ydata-profiling Integration

dev-requirements.in - Added lxml dependency for HTML parsing

renderer.py - Modified FrameProfilingRenderer to support flexible configuration options

setup.py - Updated ydata-profiling version requirement to >=2.4.0

test_renderer.py - Added comprehensive tests for ProfileReport configuration options

eapolinario avatar Dec 27 '24 19:12 eapolinario