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

Add Python Dependency Deck #2264

Merged
merged 66 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
fccb28d
feat: refactor source code rendering in `SourceCodeDeck` class
jasonlai1218 Mar 11, 2024
1fec103
refactor: refactor deck rendering for Python dependencies
jasonlai1218 Mar 12, 2024
4ec42a7
refactor: refactor class properties and initialization
jasonlai1218 Mar 12, 2024
6e70e23
- feat: consolidate Python dependencies in `flytekit/__init__.py`
jasonlai1218 Mar 12, 2024
4dfeb34
refactor: refactor `flytekit/deck/deck.py` for `pandas` compatibility
jasonlai1218 Mar 12, 2024
27ec1f2
refactor: refactor return types across multiple files
jasonlai1218 Mar 12, 2024
5fd853d
feat: consolidate Python dependency management in FlyteContext
jasonlai1218 Mar 12, 2024
bc21848
refactor: refactor code to use DataFrame for package handling
jasonlai1218 Mar 13, 2024
9b30dc6
chore: refactor code for improved naming conventions
jasonlai1218 Mar 13, 2024
5ebc980
style: improve table alignment styling in CSS
jasonlai1218 Mar 13, 2024
501fcb6
refactor: refactor method and variable names across files
jasonlai1218 Mar 13, 2024
6ce4575
refactor: refactor imports in flytekit package
jasonlai1218 Mar 13, 2024
7556126
refactor: consolidate import statements in core/context_manager.py
jasonlai1218 Mar 13, 2024
534fa0f
style: improve code consistency and error checking
jasonlai1218 Mar 13, 2024
c4358f0
refactor: refactor Python dependency handling in classes
jasonlai1218 Mar 13, 2024
363f84f
chore: optimize imports in deck.py files
jasonlai1218 Mar 13, 2024
a775c7f
test: improve test coverage for PythonDependencyDeck class
jasonlai1218 Mar 13, 2024
a4773e0
feat: enhance user space deck management
jasonlai1218 Mar 13, 2024
05db9ab
refactor: refactor deck module and unit tests
jasonlai1218 Mar 13, 2024
d2bc23b
fix: update subprocess calls to use `sys.executable`
jasonlai1218 Mar 16, 2024
099a931
feat: refactor HTML generation logic and improve user experience
jasonlai1218 Mar 16, 2024
b02db7c
feat: enhance table content copying functionality
jasonlai1218 Mar 16, 2024
aaedf7a
refactor: improve table content copying functionality
jasonlai1218 Mar 16, 2024
938e0dd
refactor: improve package management and error handling
jasonlai1218 Mar 16, 2024
265cc60
chore: standardize whitespace in requirements_txt handling
jasonlai1218 Mar 16, 2024
7e132c0
style: standardize quotation marks for package_info keys
jasonlai1218 Mar 16, 2024
9eacfe7
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Mar 16, 2024
20a3e9f
refactor: simplify requirements_txt generation
jasonlai1218 Mar 17, 2024
341a7da
docs: fix typos and improve code consistency across files
jasonlai1218 Mar 17, 2024
de90326
refactor: refactor dependency handling in PythonDependencyDeck class
jasonlai1218 Mar 18, 2024
b5f49b4
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Mar 18, 2024
5634e78
refactor: update default name and test assertion in PythonDependencyDeck
jasonlai1218 Mar 18, 2024
9bd7995
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Mar 19, 2024
79a09ac
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Mar 20, 2024
e4c0bd0
refactor: update rendering of Pandas DataFrame using MarkdownRenderer
jasonlai1218 Mar 25, 2024
0091f25
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Mar 25, 2024
5a58a8e
refactor: refactor PythonDependencyDeck and related classes
jasonlai1218 Mar 25, 2024
12ae643
feat: use `TableRenderer` for rendering DataFrames
jasonlai1218 Mar 26, 2024
16e681c
test: refactor codebase for improved performance
jasonlai1218 Mar 26, 2024
fc21591
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Mar 26, 2024
ac21522
test: update test_deck.py for python dependency deck testing
jasonlai1218 Mar 26, 2024
96a852c
style: standardize import statements for pandas in project files
jasonlai1218 Mar 26, 2024
20b84fc
refactor: refactor import statements for `TableRenderer` usage
jasonlai1218 Mar 27, 2024
8572008
refactor: update PythonDependencyRenderer description
jasonlai1218 Mar 27, 2024
1c93057
refactor: refactor type hints and assertions across files
jasonlai1218 Mar 27, 2024
a32ce2d
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Mar 27, 2024
8a40607
feat: refactor rendering classes in deck and plugins
jasonlai1218 Mar 29, 2024
f731757
docs: standardize markdown formatting across files
jasonlai1218 Mar 29, 2024
20e96c5
refactor: refactor markdown table rendering in PythonDependencyRenderer
jasonlai1218 Mar 29, 2024
352b79b
style: improve code consistency in PythonDependencyRenderer
jasonlai1218 Mar 29, 2024
0aad322
style: standardize formatting for better readability
jasonlai1218 Mar 29, 2024
7f40ad8
fix: update assertion to check for `Name` and `Version` consistency
jasonlai1218 Mar 29, 2024
1885662
test: extend test deadlines across various tests
jasonlai1218 Mar 29, 2024
3a262c4
Revert "test: extend test deadlines across various tests"
eapolinario Mar 29, 2024
1534070
Only run generate decks in python_function_task if decks are enabled
eapolinario Mar 29, 2024
03cb07c
Remove breakpoint
eapolinario Mar 29, 2024
6153065
Fix test_deck.py tests to account for the number of expected decks (n…
eapolinario Mar 29, 2024
3574aad
Increase deadline of eager tests
eapolinario Mar 29, 2024
edb7c18
Fix lint error
eapolinario Mar 30, 2024
57597ed
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Mar 30, 2024
9a2470e
test: update test assertions in test_deck.py
jasonlai1218 Mar 30, 2024
fc81271
refactor: refactor Python code organization
jasonlai1218 Apr 2, 2024
0f5a455
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Apr 2, 2024
ef47b14
docs: refactor project structure and improve user experience
jasonlai1218 Apr 3, 2024
8c9af6b
Merge branch 'master' into add-py-deps-deck
jasonlai1218 Apr 3, 2024
814a952
Separate out tests that require hypothesis
eapolinario Apr 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion flytekit/core/python_function_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,14 @@
source_code = inspect.getsource(self._task_function)

from flytekit.deck import Deck
from flytekit.deck.renderer import SourceCodeRenderer
from flytekit.deck.renderer import PythonDependencyRenderer, SourceCodeRenderer

Check warning on line 357 in flytekit/core/python_function_task.py

View check run for this annotation

Codecov / codecov/patch

flytekit/core/python_function_task.py#L357

Added line #L357 was not covered by tests

source_code_deck = Deck("Source Code")
renderer = SourceCodeRenderer()
source_code_deck.append(renderer.to_html(source_code))

python_dependencies_deck = Deck("Python Dependencies")
Copy link
Member

@thomasjpfan thomasjpfan Apr 2, 2024

Choose a reason for hiding this comment

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

Nit: Making this shorter looks better in the UI:

Suggested change
python_dependencies_deck = Deck("Python Dependencies")
python_dependencies_deck = Deck("Dependencies")

Screenshot 2024-04-02 at 3 21 58 PM

"Python Dependencies using a second line":
Screenshot 2024-04-02 at 3 23 12 PM

renderer = PythonDependencyRenderer()
python_dependencies_deck.append(renderer.to_html())

Check warning on line 365 in flytekit/core/python_function_task.py

View check run for this annotation

Codecov / codecov/patch

flytekit/core/python_function_task.py#L363-L365

Added lines #L363 - L365 were not covered by tests
pingsutw marked this conversation as resolved.
Show resolved Hide resolved

return super()._write_decks(native_inputs, native_outputs_as_map, ctx, new_user_params)
70 changes: 70 additions & 0 deletions flytekit/deck/renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,73 @@
css = formatter.get_style_defs(".highlight").replace("#fff0f0", "#ffffff")
html = highlight(source_code, PythonLexer(), formatter)
return f"<style>{css}</style>{html}"


class PythonDependencyRenderer:
"""
PythonDependencyDeck is a deck that contains information about packages installed via pip.
"""

def __init__(self, title: str = "Python Dependencies"):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def __init__(self, title: str = "Python Dependencies"):
def __init__(self, title: str = "Dependencies"):

self._title = title

Check warning on line 97 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L97

Added line #L97 was not covered by tests

def to_html(self) -> str:
import json
import subprocess
import sys

Check warning on line 102 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L100-L102

Added lines #L100 - L102 were not covered by tests

from flytekit.loggers import logger

Check warning on line 104 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L104

Added line #L104 was not covered by tests

try:
installed_packages = json.loads(

Check warning on line 107 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L106-L107

Added lines #L106 - L107 were not covered by tests
subprocess.check_output([sys.executable, "-m", "pip", "list", "--format", "json"])
)
requirements_txt = subprocess.check_output(["pip", "freeze"]).decode("utf-8").replace("\\n", "\n").rstrip()
jasonlai1218 marked this conversation as resolved.
Show resolved Hide resolved
except subprocess.CalledProcessError as e:
logger.error(f"Error occurred while fetching installed packages: {e}")
return ""

Check warning on line 113 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L110-L113

Added lines #L110 - L113 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

Having an empty HTML feels like a bad dev experience. I think it's worth having an error message:

Suggested change
return ""
return "Error occurred while fetching installed packages."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree


markdown_table = (

Check warning on line 115 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L115

Added line #L115 was not covered by tests
"<table>\n<tr>\n<th style='text-align:left;'>Name</th>\n<th style='text-align:left;'>Version</th>\n</tr>\n"
)
jasonlai1218 marked this conversation as resolved.
Show resolved Hide resolved

for entry in installed_packages:
markdown_table += f"<tr>\n<td>{entry['name']}</td>\n<td>{entry['version']}</td>\n</tr>\n"

Check warning on line 120 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L120

Added line #L120 was not covered by tests

markdown_table += "</table>"

Check warning on line 122 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L122

Added line #L122 was not covered by tests

table = MarkdownRenderer().to_html(markdown_table)
html = f"""

Check warning on line 125 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L124-L125

Added lines #L124 - L125 were not covered by tests
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Flyte Dependencies</title>
<script>
async function copyTable() {{
var requirements_txt = document.getElementById('requirements_txt');

try {{
await navigator.clipboard.writeText(requirements_txt.innerText);
}} catch (err) {{
console.log('Error accessing the clipboard: ' + err);
}}
}}
</script>
</head>
<body>

<button onclick="copyTable()">
Copy link
Member

Choose a reason for hiding this comment

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

@eapolinario This copy button does not work. I get this error in the console:

Screenshot 2024-04-02 at 3 14 42 PM

Screenshot 2024-04-02 at 3 15 38 PM

If we can not get this to work, I'm okay with removing the copy button and just showing the table.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is blocked on flyteorg/flyteconsole#852.

Copy link
Collaborator

@eapolinario eapolinario Apr 3, 2024

Choose a reason for hiding this comment

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

That just got merged, so next Flyte release is going to have this.

<span>Copy table as requirements.txt</span>
</button>
<br><br><br>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<br><br><br>
<h3>Python Dependencies</h3>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-04-03 at 11 38 04 AM


{table}

<div id="requirements_txt" style="display:none">{requirements_txt}</div>

</body>
</html>
"""
return html

Check warning on line 158 in flytekit/deck/renderer.py

View check run for this annotation

Codecov / codecov/patch

flytekit/deck/renderer.py#L158

Added line #L158 was not covered by tests
39 changes: 31 additions & 8 deletions tests/flytekit/unit/deck/test_deck.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from flytekit import Deck, FlyteContextManager, task
from flytekit.deck import MarkdownRenderer, SourceCodeRenderer, TopFrameRenderer
from flytekit.deck.deck import _output_deck
from flytekit.deck.renderer import PythonDependencyRenderer


@pytest.mark.skipif("pandas" not in sys.modules, reason="Pandas is not installed.")
Expand Down Expand Up @@ -50,9 +51,9 @@ def test_timeline_deck():
@pytest.mark.parametrize(
"disable_deck,expected_decks",
[
(None, 2), # time line deck + source code deck
(False, 4), # time line deck + source code deck + input and output decks
(True, 2), # time line deck + source code deck
(None, 3), # time line deck + source code deck + python dependency deck
(False, 5), # time line deck + source code deck + python dependency deck + input and output decks
(True, 3), # time line deck + source code deck + python dependency deck
],
)
def test_deck_for_task(disable_deck, expected_decks):
Expand All @@ -75,11 +76,21 @@ def t1(a: int) -> str:
@pytest.mark.parametrize(
"enable_deck,disable_deck, expected_decks, expect_error",
[
(None, None, 3, False), # default deck and time line deck + source code deck
(None, False, 5, False), # default deck and time line deck + source code deck + input and output decks
(None, True, 3, False), # default deck and time line deck + source code deck
(True, None, 5, False), # default deck and time line deck + source code deck + input and output decks
(False, None, 3, False), # default deck and time line deck + source code deck
(None, None, 4, False), # default deck and time line deck + source code deck + python dependency deck
(
None,
False,
6,
False,
), # default deck and time line deck + source code deck + python dependency deck + input and output decks
(None, True, 4, False), # default deck and time line deck + source code deck + python dependency deck
(
True,
None,
6,
False,
), # default deck and time line deck + source code deck + python dependency deck + input and output decks
(False, None, 4, False), # default deck and time line deck + source code deck + python dependency deck
(True, True, -1, True), # Set both disable_deck and enable_deck to True and confirm that it fails
(False, False, -1, True), # Set both disable_deck and enable_deck to False and confirm that it fails
],
Expand Down Expand Up @@ -176,3 +187,15 @@ def test_source_code_renderer():
# Assert that the color #ffffff is used instead of #fff0f0
assert "#ffffff" in result
assert "#fff0f0" not in result


def test_python_dependency_renderer():
renderer = PythonDependencyRenderer()
result = renderer.to_html()

# Assert that the result includes parts of the python dependency
assert "name" in result
assert "version" in result
pingsutw marked this conversation as resolved.
Show resolved Hide resolved

# Assert that the button of copy
assert 'button onclick="copyTable()"' in result
Loading