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

Conversation

jasonlai1218
Copy link
Contributor

@jasonlai1218 jasonlai1218 commented Mar 13, 2024

Tracking issue

Why are the changes needed?

In Python Dependency Deck, the version of the Python package currently being used can be displayed, which can make debugging easier.

What changes were proposed in this pull request?

  • Add a class PythonDependencyDeck to deck.py
  • Add a method add_deck to FlyteContext class

How was this patch tested?

Setup process

import flytekit
from flytekit import task, workflow, ImageSpec

flytekit_version = "git+https://github.com/jasonlai1218/flytekit.git@add-py-deps-deck#commit=b5f49b4070621ba87224c9d223a6f7d710084673"
deck_plugin_version = "git+https://github.com/jasonlai1218/flytekit.git@add-py-deps-deck#commit=b5f49b4070621ba87224c9d223a6f7d710084673#subdirectory=plugins/flytekit-deck-standard"

image_spec = ImageSpec(
    packages=[flytekit_version, deck_plugin_version],
    apt_packages=["git"],
    registry="jasonlai1218",
    env={"FLYTE_SDK_LOGGING_LEVEL": "20"},
)


@task(container_image=image_spec, enable_deck=True)
def python_dependency_deck() -> None:
    ctx = flytekit.FlyteContextManager.current_context()
    ctx.add_deck(PythonDependencyDeck())


@workflow
def wf() -> None:
    python_dependency_deck()


if __name__ == "__main__":
    wf()

Screenshots

Screenshot 2024-04-03 at 11 38 04 AM

click copy button and paste result:

adlfs==2024.2.0
aiobotocore==2.12.1
aiohttp==3.9.3
aioitertools==0.11.0
aiosignal==1.3.1
annotated-types==0.6.0
arrow==1.3.0
asttokens==2.4.1
attrs==23.2.0
azure-core==1.30.1
azure-datalake-store==0.0.53
azure-identity==1.15.0
azure-storage-blob==12.19.1
binaryornot==0.4.4
botocore==1.34.51
...
...
...
websocket-client==1.7.0
widgetsnbextension==4.0.10
wordcloud==1.9.3
wrapt==1.16.0
yarl==1.9.4
ydata-profiling==4.7.0
zipp==3.18.1

Check all the applicable boxes

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

Related PRs

flyteorg/flyteconsole#852

Docs link

- Add a new class `SourceCodeDeck` to render the source code of a task
- Implement the `html` property in the `SourceCodeDeck` class

Signed-off-by: jason.lai <[email protected]>
- Change the class name from `SourceCodeDeck` to `PythonDependencyDeck`
- Update the deck description to reflect python dependencies instead of source code
- Remove the source code rendering functionality and replace it with a table renderer for python dependencies
- Add subprocess logic to fetch installed python packages and render them in an HTML table

Signed-off-by: jason.lai <[email protected]>
- Add a property method for `pythondependency_deck`
- Initialize `python_dependency_deck` if it is None
- Remove a property method for `timeline_deck`

Signed-off-by: jason.lai <[email protected]>
- Add `PythonDependencyDeck` to `flytekit/__init__.py`

Signed-off-by: jason.lai <[email protected]>
- Add `pandas` import to `flytekit/deck/deck.py`
- Change the input of `TableRenderer().to_html` to a `pd.DataFrame` in `flytekit/deck/deck.py`

Signed-off-by: jason.lai <[email protected]>
- Update the return type annotation for `pythondependency_deck` in `context_manager.py`

Signed-off-by: jason.lai <[email protected]>
- Add `PythonDependencyDeck` to the TYPE_CHECKING import
- Add a method `add_deck` to `FlyteContext` class

Signed-off-by: jason.lai <[email protected]>
- Refactor the code to use a DataFrame for installed packages handling

Signed-off-by: jason.lai <[email protected]>
- Update the name of the PythonDependencyDeck instance from "PythonDependencyDeck" to "Python Dependency"

Signed-off-by: jason.lai <[email protected]>
- Add CSS style to center align the table content

Signed-off-by: jason.lai <[email protected]>
- Rename method `pythondependency_deck` to `python_dependency_deck`
- Update variable names in method `python_dependency_deck`
- Update method comments in class `FlyteContext`
- Update method comments in class `PythonDependencyDeck`

Signed-off-by: jason.lai <[email protected]>
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Mar 13, 2024
- Remove import of `PythonDependencyDeck` from `flytekit/__init__.py`

Signed-off-by: jason.lai <[email protected]>
- Remove the import of `PythonDependencyDeck` from `flytekit/core/context_manager.py`
- Add an import of `Deck` to `flytekit/core/context_manager.py`

Signed-off-by: jason.lai <[email protected]>
- Add a condition to check for deck existence before appending
- Change single quotes to double quotes for consistency
- Update package split delimiter to double quotes

Signed-off-by: jason.lai <[email protected]>
- Remove the `python_dependency_deck` property from the `ExecutionParameters` class in `context_manager.py`
- Update the `__init__` method signature in the `PythonDependencyDeck` class in `deck.py`

Signed-off-by: jason.lai <[email protected]>
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 90.32258% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 82.68%. Comparing base (55f0b19) to head (814a952).

Files Patch % Lines
flytekit/deck/renderer.py 85.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2264      +/-   ##
==========================================
- Coverage   83.50%   82.68%   -0.83%     
==========================================
  Files         324      305      -19     
  Lines       24754    24063     -691     
  Branches     3734     3523     -211     
==========================================
- Hits        20672    19896     -776     
- Misses       3452     3542      +90     
+ Partials      630      625       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Remove unnecessary import of `pandas` in `flytekit/deck/deck.py`
- Add import of `pandas` in `flytekit/deck/deck.py`

Signed-off-by: jason.lai <[email protected]>
flytekit/deck/deck.py Outdated Show resolved Hide resolved
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

flytekit/deck/deck.py Outdated Show resolved Hide resolved
flytekit/deck/deck.py Outdated Show resolved Hide resolved
flytekit/deck/deck.py Outdated Show resolved Hide resolved
- Add a new test for the PythonDependencyDeck class in test_deck.py
- Add assertions for specific strings in the HTML content in the test_python_dependency_deck() function

Signed-off-by: jason.lai <[email protected]>
- Clear user space decks before adding a new deck
- Ensure only one deck is added to user space params

Signed-off-by: jason.lai <[email protected]>
- Import json and TableRenderer in `flytekit/deck/deck.py`
- Change how installed packages are fetched in `flytekit/deck/deck.py`
- Remove unused code in `flytekit/deck/deck.py`
- Remove assertions for "Library" and "Version" in `tests/flytekit/unit/deck/test_deck.py`

Signed-off-by: jason.lai <[email protected]>
flytekit/deck/deck.py Outdated Show resolved Hide resolved
flytekit/deck/deck.py Outdated Show resolved Hide resolved
- Modify the table headers to have left-aligned text
- Add extra line breaks after the button element

Signed-off-by: jason.lai <[email protected]>
jasonlai1218 and others added 6 commits March 29, 2024 23:24
- Change the assertion to check for `Name` and `Version` instead of `name` and `version` in the result

Signed-off-by: jason.lai <[email protected]>
- Increase the test deadlines from 2 to 20 seconds in test_type_conversion_errors.py
- Update test deadlines from 2 to 40 seconds in test_type_conversion_errors.py
- Adjust test deadlines in test_eager_workflows.py for various tests

Signed-off-by: jason.lai <[email protected]>
This reverts commit 1885662.

Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
…ow that we're no longer writing decks unnecessarily)

Signed-off-by: Eduardo Apolinario <[email protected]>
eapolinario and others added 4 commits March 29, 2024 16:41
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
- Add a patch to the `test_python_dependency_renderer` function
- Include assertions for `numpy` and `1.21.0` in the test result
- Update the `test_deck.py` file in the `tests/flytekit/unit/deck` directory

Signed-off-by: jason.lai <[email protected]>
flytekit/core/python_function_task.py Outdated Show resolved Hide resolved
flytekit/deck/renderer.py Outdated Show resolved Hide resolved
flytekit/deck/renderer.py Outdated Show resolved Hide resolved
- Import `PythonDependencyRenderer` and `SourceCodeRenderer` in `PythonFunctionTask` from separate files
- Modify the way `requirements_txt` is generated in `PythonDependencyRenderer`
- Refactor the creation of the `table` variable in `PythonDependencyRenderer` to improve readability

Signed-off-by: jason.lai <[email protected]>
)
except subprocess.CalledProcessError as e:
logger.error(f"Error occurred while fetching installed packages: {e}")
return ""
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

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

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

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"):

<button onclick="copyTable()">
<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

- Change the title of the deck from `Python Dependencies` to `Dependencies`
- Update the error message in case of a subprocess error in fetching installed packages
- Add a heading `Python Dependencies` above the table output in the HTML

Signed-off-by: jason.lai <[email protected]>
thomasjpfan
thomasjpfan previously approved these changes Apr 3, 2024
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Assuming https://github.com/flyteorg/flytekit/pull/2264/files#r1548469361 will get resolved with a new flyte release, this PR LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by maintainer label Apr 3, 2024
@eapolinario
Copy link
Collaborator

eapolinario commented Apr 3, 2024

FYI: I pushed a commit to handle tests that require hypothesis slightly differently. Now we run them via a separate make target (called unit_test_hypothesis) where we control some of the hyperparameters like deadline and max_examples using a profile (as per the docs).

@eapolinario eapolinario merged commit eac772a into flyteorg:master Apr 3, 2024
48 of 49 checks passed
ChungYujoyce pushed a commit to ChungYujoyce/flytekit that referenced this pull request Apr 5, 2024
* feat: refactor source code rendering in `SourceCodeDeck` class

- Add a new class `SourceCodeDeck` to render the source code of a task
- Implement the `html` property in the `SourceCodeDeck` class

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor deck rendering for Python dependencies

- Change the class name from `SourceCodeDeck` to `PythonDependencyDeck`
- Update the deck description to reflect python dependencies instead of source code
- Remove the source code rendering functionality and replace it with a table renderer for python dependencies
- Add subprocess logic to fetch installed python packages and render them in an HTML table

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor class properties and initialization

- Add a property method for `pythondependency_deck`
- Initialize `python_dependency_deck` if it is None
- Remove a property method for `timeline_deck`

Signed-off-by: jason.lai <[email protected]>

* - feat: consolidate Python dependencies in `flytekit/__init__.py`

- Add `PythonDependencyDeck` to `flytekit/__init__.py`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor `flytekit/deck/deck.py` for `pandas` compatibility

- Add `pandas` import to `flytekit/deck/deck.py`
- Change the input of `TableRenderer().to_html` to a `pd.DataFrame` in `flytekit/deck/deck.py`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor return types across multiple files

- Update the return type annotation for `pythondependency_deck` in `context_manager.py`

Signed-off-by: jason.lai <[email protected]>

* feat: consolidate Python dependency management in FlyteContext

- Add `PythonDependencyDeck` to the TYPE_CHECKING import
- Add a method `add_deck` to `FlyteContext` class

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor code to use DataFrame for package handling

- Refactor the code to use a DataFrame for installed packages handling

Signed-off-by: jason.lai <[email protected]>

* chore: refactor code for improved naming conventions

- Update the name of the PythonDependencyDeck instance from "PythonDependencyDeck" to "Python Dependency"

Signed-off-by: jason.lai <[email protected]>

* style: improve table alignment styling in CSS

- Add CSS style to center align the table content

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor method and variable names across files

- Rename method `pythondependency_deck` to `python_dependency_deck`
- Update variable names in method `python_dependency_deck`
- Update method comments in class `FlyteContext`
- Update method comments in class `PythonDependencyDeck`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor imports in flytekit package

- Remove import of `PythonDependencyDeck` from `flytekit/__init__.py`

Signed-off-by: jason.lai <[email protected]>

* refactor: consolidate import statements in core/context_manager.py

- Remove the import of `PythonDependencyDeck` from `flytekit/core/context_manager.py`
- Add an import of `Deck` to `flytekit/core/context_manager.py`

Signed-off-by: jason.lai <[email protected]>

* style: improve code consistency and error checking

- Add a condition to check for deck existence before appending
- Change single quotes to double quotes for consistency
- Update package split delimiter to double quotes

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor Python dependency handling in classes

- Remove the `python_dependency_deck` property from the `ExecutionParameters` class in `context_manager.py`
- Update the `__init__` method signature in the `PythonDependencyDeck` class in `deck.py`

Signed-off-by: jason.lai <[email protected]>

* chore: optimize imports in deck.py files

- Remove unnecessary import of `pandas` in `flytekit/deck/deck.py`
- Add import of `pandas` in `flytekit/deck/deck.py`

Signed-off-by: jason.lai <[email protected]>

* test: improve test coverage for PythonDependencyDeck class

- Add a new test for the PythonDependencyDeck class in test_deck.py
- Add assertions for specific strings in the HTML content in the test_python_dependency_deck() function

Signed-off-by: jason.lai <[email protected]>

* feat: enhance user space deck management

- Clear user space decks before adding a new deck
- Ensure only one deck is added to user space params

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor deck module and unit tests

- Import json and TableRenderer in `flytekit/deck/deck.py`
- Change how installed packages are fetched in `flytekit/deck/deck.py`
- Remove unused code in `flytekit/deck/deck.py`
- Remove assertions for "Library" and "Version" in `tests/flytekit/unit/deck/test_deck.py`

Signed-off-by: jason.lai <[email protected]>

* fix: update subprocess calls to use `sys.executable`

- Import `sys` to fix an issue with subprocess execution
- Update the subprocess call to use `sys.executable` instead of `pip`

Signed-off-by: jason.lai <[email protected]>

* feat: refactor HTML generation logic and improve user experience

- Update the HTML generation logic to include a button for copying the table as requirements.txt.

Signed-off-by: jason.lai <[email protected]>

* feat: enhance table content copying functionality

- Add functionality to copy table content as requirements.txt
- Improve error handling when copying table content
- Display table content as hidden div for copying

Signed-off-by: jason.lai <[email protected]>

* refactor: improve table content copying functionality

- Remove unnecessary code for table content copying
- Update table content copying functionality to use innerText of requirements_txt element

Signed-off-by: jason.lai <[email protected]>

* refactor: improve package management and error handling

- Add logic to generate a `requirements.txt` file from installed packages
- Update error logging message in case of subprocess error
- Update console log message when accessing clipboard
- Update `requirements_txt` div content with actual `requirements_txt` variable

Signed-off-by: jason.lai <[email protected]>

* chore: standardize whitespace in requirements_txt handling

- Remove trailing whitespace from `requirements_txt` string
- Add a whitespace to the end of the `requirements_txt` variable
- Log an error message when fetching installed packages fails

Signed-off-by: jason.lai <[email protected]>

* style: standardize quotation marks for package_info keys

- Corrected quotation marks in package_info keys
- Changed single quotes to double quotes for consistency

Signed-off-by: jason.lai <[email protected]>

* refactor: simplify requirements_txt generation

- Refactor code to simplify the generation of `requirements_txt`
- Add the output of `pip freeze` to `requirements_txt`

Signed-off-by: jason.lai <[email protected]>

* docs: fix typos and improve code consistency across files

- Fix a typo in the usage of the `pip freeze` command in the PythonDependencyDeck class

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor dependency handling in PythonDependencyDeck class

- Update the way `requirements_txt` is populated in the `PythonDependencyDeck` class

Signed-off-by: jason.lai <[email protected]>

* refactor: update default name and test assertion in PythonDependencyDeck

- Update the default name in the PythonDependencyDeck constructor from "Python Dependency" to "Python Dependencies"
- Update the assertion in the test_python_dependency_deck function to check for the new default name "Python Dependencies"

Signed-off-by: jason.lai <[email protected]>

* refactor: update rendering of Pandas DataFrame using MarkdownRenderer

- Import `MarkdownRenderer` from `flytekit.deck` instead of `TableRenderer`
- Render the Pandas DataFrame as markdown using `MarkdownRenderer` instead of `TableRenderer`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor PythonDependencyDeck and related classes

- Remove the `add_deck` method from `FlyteContext`
- Add imports for `PythonDependencyRenderer` and `PythonDependencyDeck` in `PythonFunctionTask`
- Remove the `PythonDependencyDeck` class and related methods from the `deck.py` file
- Add the `PythonDependencyRenderer` class in `renderer.py`

Signed-off-by: jason.lai <[email protected]>

* feat: use `TableRenderer` for rendering DataFrames

- Import `TableRenderer` from a different module
- Replace the `MarkdownRenderer` with `TableRenderer` to render a DataFrame as a table

Signed-off-by: jason.lai <[email protected]>

* test: refactor codebase for improved performance

- Remove unused import statements in `flytekit/deck/renderer.py`
- Add a new test for `PythonDependencyRenderer` in `tests/flytekit/unit/deck/test_deck.py`

Signed-off-by: jason.lai <[email protected]>

* test: update test_deck.py for python dependency deck testing

- Update test_deck.py to include python dependency deck in various test cases
- Adjust expected deck counts in test cases to reflect the changes
- Add test cases for scenarios involving python dependency deck and input and output decks

Signed-off-by: jason.lai <[email protected]>

* style: standardize import statements for pandas in project files

- Remove `import pandas as pd` and replace it with `import pandas as pandas`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor import statements for `TableRenderer` usage

- Import `TableRenderer` from a different location in `flytekit/deck/renderer.py`
- Remove `TableRenderer` from the imports in `flytekit-deck-standard/flytekitplugins/deck/__init__.py`

Signed-off-by: jason.lai <[email protected]>

* refactor: update PythonDependencyRenderer description

- Update the description of PythonDependencyDeck in PythonDependencyRenderer

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor type hints and assertions across files

- Change the type hint for `df` parameter to `pandas.DataFrame`
- Add an assertion to check the type of `df` is `pandas.DataFrame`

Signed-off-by: jason.lai <[email protected]>

* feat: refactor rendering classes in deck and plugins

- Add TableRenderer to TimeLineDeck class in deck.py
- Remove TableRenderer from deck/renderer.py
- Add TableRenderer to PythonDependencyRenderer in deck/renderer.py
- Add TableRenderer to flytekit-deck-standard plugin
- Remove TableRenderer from test_renderer.py

Signed-off-by: jason.lai <[email protected]>

* docs: standardize markdown formatting across files

- Update the markdown table format in PythonDependencyRenderer class

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor markdown table rendering in PythonDependencyRenderer

- Update the markdown table format in the PythonDependencyRenderer class to use HTML table tags
- Modify the markdown_table variable concatenation for installed packages entries in the PythonDependencyRenderer class

Signed-off-by: jason.lai <[email protected]>

* style: improve code consistency in PythonDependencyRenderer

- Add a line break after the button element in PythonDependencyRenderer

Signed-off-by: jason.lai <[email protected]>

* style: standardize formatting for better readability

- Modify the table headers to have left-aligned text
- Add extra line breaks after the button element

Signed-off-by: jason.lai <[email protected]>

* fix: update assertion to check for `Name` and `Version` consistency

- Change the assertion to check for `Name` and `Version` instead of `name` and `version` in the result

Signed-off-by: jason.lai <[email protected]>

* test: extend test deadlines across various tests

- Increase the test deadlines from 2 to 20 seconds in test_type_conversion_errors.py
- Update test deadlines from 2 to 40 seconds in test_type_conversion_errors.py
- Adjust test deadlines in test_eager_workflows.py for various tests

Signed-off-by: jason.lai <[email protected]>

* Revert "test: extend test deadlines across various tests"

This reverts commit 1885662.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Only run generate decks in python_function_task if decks are enabled

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove breakpoint

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix test_deck.py tests to account for the number of expected decks (now that we're no longer writing decks unnecessarily)

Signed-off-by: Eduardo Apolinario <[email protected]>

* Increase deadline of eager tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix lint error

Signed-off-by: Eduardo Apolinario <[email protected]>

* test: update test assertions in test_deck.py

- Add a patch to the `test_python_dependency_renderer` function
- Include assertions for `numpy` and `1.21.0` in the test result
- Update the `test_deck.py` file in the `tests/flytekit/unit/deck` directory

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor Python code organization

- Import `PythonDependencyRenderer` and `SourceCodeRenderer` in `PythonFunctionTask` from separate files
- Modify the way `requirements_txt` is generated in `PythonDependencyRenderer`
- Refactor the creation of the `table` variable in `PythonDependencyRenderer` to improve readability

Signed-off-by: jason.lai <[email protected]>

* docs: refactor project structure and improve user experience

- Change the title of the deck from `Python Dependencies` to `Dependencies`
- Update the error message in case of a subprocess error in fetching installed packages
- Add a heading `Python Dependencies` above the table output in the HTML

Signed-off-by: jason.lai <[email protected]>

* Separate out tests that require hypothesis

Signed-off-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: jason.lai <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
fiedlerNr9 pushed a commit that referenced this pull request Jul 25, 2024
* feat: refactor source code rendering in `SourceCodeDeck` class

- Add a new class `SourceCodeDeck` to render the source code of a task
- Implement the `html` property in the `SourceCodeDeck` class

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor deck rendering for Python dependencies

- Change the class name from `SourceCodeDeck` to `PythonDependencyDeck`
- Update the deck description to reflect python dependencies instead of source code
- Remove the source code rendering functionality and replace it with a table renderer for python dependencies
- Add subprocess logic to fetch installed python packages and render them in an HTML table

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor class properties and initialization

- Add a property method for `pythondependency_deck`
- Initialize `python_dependency_deck` if it is None
- Remove a property method for `timeline_deck`

Signed-off-by: jason.lai <[email protected]>

* - feat: consolidate Python dependencies in `flytekit/__init__.py`

- Add `PythonDependencyDeck` to `flytekit/__init__.py`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor `flytekit/deck/deck.py` for `pandas` compatibility

- Add `pandas` import to `flytekit/deck/deck.py`
- Change the input of `TableRenderer().to_html` to a `pd.DataFrame` in `flytekit/deck/deck.py`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor return types across multiple files

- Update the return type annotation for `pythondependency_deck` in `context_manager.py`

Signed-off-by: jason.lai <[email protected]>

* feat: consolidate Python dependency management in FlyteContext

- Add `PythonDependencyDeck` to the TYPE_CHECKING import
- Add a method `add_deck` to `FlyteContext` class

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor code to use DataFrame for package handling

- Refactor the code to use a DataFrame for installed packages handling

Signed-off-by: jason.lai <[email protected]>

* chore: refactor code for improved naming conventions

- Update the name of the PythonDependencyDeck instance from "PythonDependencyDeck" to "Python Dependency"

Signed-off-by: jason.lai <[email protected]>

* style: improve table alignment styling in CSS

- Add CSS style to center align the table content

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor method and variable names across files

- Rename method `pythondependency_deck` to `python_dependency_deck`
- Update variable names in method `python_dependency_deck`
- Update method comments in class `FlyteContext`
- Update method comments in class `PythonDependencyDeck`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor imports in flytekit package

- Remove import of `PythonDependencyDeck` from `flytekit/__init__.py`

Signed-off-by: jason.lai <[email protected]>

* refactor: consolidate import statements in core/context_manager.py

- Remove the import of `PythonDependencyDeck` from `flytekit/core/context_manager.py`
- Add an import of `Deck` to `flytekit/core/context_manager.py`

Signed-off-by: jason.lai <[email protected]>

* style: improve code consistency and error checking

- Add a condition to check for deck existence before appending
- Change single quotes to double quotes for consistency
- Update package split delimiter to double quotes

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor Python dependency handling in classes

- Remove the `python_dependency_deck` property from the `ExecutionParameters` class in `context_manager.py`
- Update the `__init__` method signature in the `PythonDependencyDeck` class in `deck.py`

Signed-off-by: jason.lai <[email protected]>

* chore: optimize imports in deck.py files

- Remove unnecessary import of `pandas` in `flytekit/deck/deck.py`
- Add import of `pandas` in `flytekit/deck/deck.py`

Signed-off-by: jason.lai <[email protected]>

* test: improve test coverage for PythonDependencyDeck class

- Add a new test for the PythonDependencyDeck class in test_deck.py
- Add assertions for specific strings in the HTML content in the test_python_dependency_deck() function

Signed-off-by: jason.lai <[email protected]>

* feat: enhance user space deck management

- Clear user space decks before adding a new deck
- Ensure only one deck is added to user space params

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor deck module and unit tests

- Import json and TableRenderer in `flytekit/deck/deck.py`
- Change how installed packages are fetched in `flytekit/deck/deck.py`
- Remove unused code in `flytekit/deck/deck.py`
- Remove assertions for "Library" and "Version" in `tests/flytekit/unit/deck/test_deck.py`

Signed-off-by: jason.lai <[email protected]>

* fix: update subprocess calls to use `sys.executable`

- Import `sys` to fix an issue with subprocess execution
- Update the subprocess call to use `sys.executable` instead of `pip`

Signed-off-by: jason.lai <[email protected]>

* feat: refactor HTML generation logic and improve user experience

- Update the HTML generation logic to include a button for copying the table as requirements.txt.

Signed-off-by: jason.lai <[email protected]>

* feat: enhance table content copying functionality

- Add functionality to copy table content as requirements.txt
- Improve error handling when copying table content
- Display table content as hidden div for copying

Signed-off-by: jason.lai <[email protected]>

* refactor: improve table content copying functionality

- Remove unnecessary code for table content copying
- Update table content copying functionality to use innerText of requirements_txt element

Signed-off-by: jason.lai <[email protected]>

* refactor: improve package management and error handling

- Add logic to generate a `requirements.txt` file from installed packages
- Update error logging message in case of subprocess error
- Update console log message when accessing clipboard
- Update `requirements_txt` div content with actual `requirements_txt` variable

Signed-off-by: jason.lai <[email protected]>

* chore: standardize whitespace in requirements_txt handling

- Remove trailing whitespace from `requirements_txt` string
- Add a whitespace to the end of the `requirements_txt` variable
- Log an error message when fetching installed packages fails

Signed-off-by: jason.lai <[email protected]>

* style: standardize quotation marks for package_info keys

- Corrected quotation marks in package_info keys
- Changed single quotes to double quotes for consistency

Signed-off-by: jason.lai <[email protected]>

* refactor: simplify requirements_txt generation

- Refactor code to simplify the generation of `requirements_txt`
- Add the output of `pip freeze` to `requirements_txt`

Signed-off-by: jason.lai <[email protected]>

* docs: fix typos and improve code consistency across files

- Fix a typo in the usage of the `pip freeze` command in the PythonDependencyDeck class

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor dependency handling in PythonDependencyDeck class

- Update the way `requirements_txt` is populated in the `PythonDependencyDeck` class

Signed-off-by: jason.lai <[email protected]>

* refactor: update default name and test assertion in PythonDependencyDeck

- Update the default name in the PythonDependencyDeck constructor from "Python Dependency" to "Python Dependencies"
- Update the assertion in the test_python_dependency_deck function to check for the new default name "Python Dependencies"

Signed-off-by: jason.lai <[email protected]>

* refactor: update rendering of Pandas DataFrame using MarkdownRenderer

- Import `MarkdownRenderer` from `flytekit.deck` instead of `TableRenderer`
- Render the Pandas DataFrame as markdown using `MarkdownRenderer` instead of `TableRenderer`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor PythonDependencyDeck and related classes

- Remove the `add_deck` method from `FlyteContext`
- Add imports for `PythonDependencyRenderer` and `PythonDependencyDeck` in `PythonFunctionTask`
- Remove the `PythonDependencyDeck` class and related methods from the `deck.py` file
- Add the `PythonDependencyRenderer` class in `renderer.py`

Signed-off-by: jason.lai <[email protected]>

* feat: use `TableRenderer` for rendering DataFrames

- Import `TableRenderer` from a different module
- Replace the `MarkdownRenderer` with `TableRenderer` to render a DataFrame as a table

Signed-off-by: jason.lai <[email protected]>

* test: refactor codebase for improved performance

- Remove unused import statements in `flytekit/deck/renderer.py`
- Add a new test for `PythonDependencyRenderer` in `tests/flytekit/unit/deck/test_deck.py`

Signed-off-by: jason.lai <[email protected]>

* test: update test_deck.py for python dependency deck testing

- Update test_deck.py to include python dependency deck in various test cases
- Adjust expected deck counts in test cases to reflect the changes
- Add test cases for scenarios involving python dependency deck and input and output decks

Signed-off-by: jason.lai <[email protected]>

* style: standardize import statements for pandas in project files

- Remove `import pandas as pd` and replace it with `import pandas as pandas`

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor import statements for `TableRenderer` usage

- Import `TableRenderer` from a different location in `flytekit/deck/renderer.py`
- Remove `TableRenderer` from the imports in `flytekit-deck-standard/flytekitplugins/deck/__init__.py`

Signed-off-by: jason.lai <[email protected]>

* refactor: update PythonDependencyRenderer description

- Update the description of PythonDependencyDeck in PythonDependencyRenderer

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor type hints and assertions across files

- Change the type hint for `df` parameter to `pandas.DataFrame`
- Add an assertion to check the type of `df` is `pandas.DataFrame`

Signed-off-by: jason.lai <[email protected]>

* feat: refactor rendering classes in deck and plugins

- Add TableRenderer to TimeLineDeck class in deck.py
- Remove TableRenderer from deck/renderer.py
- Add TableRenderer to PythonDependencyRenderer in deck/renderer.py
- Add TableRenderer to flytekit-deck-standard plugin
- Remove TableRenderer from test_renderer.py

Signed-off-by: jason.lai <[email protected]>

* docs: standardize markdown formatting across files

- Update the markdown table format in PythonDependencyRenderer class

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor markdown table rendering in PythonDependencyRenderer

- Update the markdown table format in the PythonDependencyRenderer class to use HTML table tags
- Modify the markdown_table variable concatenation for installed packages entries in the PythonDependencyRenderer class

Signed-off-by: jason.lai <[email protected]>

* style: improve code consistency in PythonDependencyRenderer

- Add a line break after the button element in PythonDependencyRenderer

Signed-off-by: jason.lai <[email protected]>

* style: standardize formatting for better readability

- Modify the table headers to have left-aligned text
- Add extra line breaks after the button element

Signed-off-by: jason.lai <[email protected]>

* fix: update assertion to check for `Name` and `Version` consistency

- Change the assertion to check for `Name` and `Version` instead of `name` and `version` in the result

Signed-off-by: jason.lai <[email protected]>

* test: extend test deadlines across various tests

- Increase the test deadlines from 2 to 20 seconds in test_type_conversion_errors.py
- Update test deadlines from 2 to 40 seconds in test_type_conversion_errors.py
- Adjust test deadlines in test_eager_workflows.py for various tests

Signed-off-by: jason.lai <[email protected]>

* Revert "test: extend test deadlines across various tests"

This reverts commit 1885662.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Only run generate decks in python_function_task if decks are enabled

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove breakpoint

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix test_deck.py tests to account for the number of expected decks (now that we're no longer writing decks unnecessarily)

Signed-off-by: Eduardo Apolinario <[email protected]>

* Increase deadline of eager tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix lint error

Signed-off-by: Eduardo Apolinario <[email protected]>

* test: update test assertions in test_deck.py

- Add a patch to the `test_python_dependency_renderer` function
- Include assertions for `numpy` and `1.21.0` in the test result
- Update the `test_deck.py` file in the `tests/flytekit/unit/deck` directory

Signed-off-by: jason.lai <[email protected]>

* refactor: refactor Python code organization

- Import `PythonDependencyRenderer` and `SourceCodeRenderer` in `PythonFunctionTask` from separate files
- Modify the way `requirements_txt` is generated in `PythonDependencyRenderer`
- Refactor the creation of the `table` variable in `PythonDependencyRenderer` to improve readability

Signed-off-by: jason.lai <[email protected]>

* docs: refactor project structure and improve user experience

- Change the title of the deck from `Python Dependencies` to `Dependencies`
- Update the error message in case of a subprocess error in fetching installed packages
- Add a heading `Python Dependencies` above the table output in the HTML

Signed-off-by: jason.lai <[email protected]>

* Separate out tests that require hypothesis

Signed-off-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: jason.lai <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Jan Fiedler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants