Core/enable deck

[novahow]

Tracking issue

flyteorg/flyte#5095

closes #5095

Why are the changes needed?

Make the ui display deck by default, while some users don't want to render large input/output such as dataframes
let users customize displayed decks.

What changes were proposed in this pull request?

1. add an parameter to task: decks, which takes a tuple of str and elements should be one of ["input", "output", "dependencies", "source_code", "Timeline"]
2. maintains compatibility with deprecated disable_deck params
   3. Currently by default, the flyteconsole will display Timeline and Source Code deck.
3. utilize a ExecutionParameters builder pattern to inject expected decks in dispatch_execute
4. Added the output_metadata_prefix param to builder of ExecutionParameters, not sure why it wasn't there initially.

How was this patch tested?

unit tests

Setup process

Screenshots

code snippet:

from math import sqrt
from flytekit import workflow, task, LaunchPlan
from typing import List, Union
import random
import pdb

@task(enable_deck=False)
def standard_deviation(values: List[float], mu: float) -> float:
    variance = sum([(x - mu) ** 2 for x in values])
    return sqrt(variance)

@task(disable_deck=False)
def standard_scale(values: List[float], mu: float, sigma: float) -> List[float]:
    return [(x - mu) / sigma for x in values]

from flytekit import workflow
from flytekit.types.file import FlyteFile
from flytekitplugins.deck.renderer import ImageRenderer


from flytekit.deck.deck import Deck, DeckFields
@task(decks=[DeckFields.INPUT.value, DeckFields.TIMELINE.value, DeckFields.DEPENDENCIES.value], enable_deck=True)
def image_renderer(image: FlyteFile) -> None:
    Deck("Image Renderer", ImageRenderer().to_html(image_src=image))


@task(decks=[DeckFields.OUTPUT])
def mean(values: List[float]) -> float:
    return sum(values) / len(values)

@task
def mean2(values: List[float]) -> float:
    return sum(values) / len(values)

@task
def generate_data(num_samples: int, seed: int) -> List[float]:
    random.seed(seed)
    return [random.random() for _ in range(num_samples)]


@workflow
def standard_scale_workflow(values: List[float]) -> List[float]:
    mu = mean(values=values)
    _ = image_renderer(image="https://bit.ly/3KZ95q4")
    sigma = standard_deviation(values=values, mu=mu)
    return standard_scale(values=values, mu=mu, sigma=sigma)


@workflow
def workflow_with_subworkflow(num_samples: int, seed: int) -> List[float]:
    data = generate_data(num_samples=num_samples, seed=seed)
    return standard_scale_workflow(values=data)


# pdb.set_trace()
print("Compiled")
# LaunchPlan.CACHE.pop("standard_scale_lp", None)
standard_scale_launch_plan = LaunchPlan.get_or_create(
    standard_scale_workflow,
    name="standard_scale_lp",
    default_inputs={"values": [3.0, 4.0, 5.0]}
)


if __name__ == "__main__":
    print(standard_scale_launch_plan())

enable_deck is None, decks ignored

enable_deck=True, specified decks+custom image deck

enable_deck=False

disable_deck=False, displaying default decks(source_code, dependencies)

Check all the applicable boxes

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

Related PRs

Docs link

[kumare3]

I do not prefer default decks. There are security and performance implications. Can we enable these decks all using the same flag

If enable_deck is true then all, else you can pass a deck selection object to it

[novahow]

I do not prefer default decks. There are security and performance implications. Can we enable these decks all using the same flag

If enable_deck is true then all, else you can pass a deck selection object to it

Sure. Additionally, could you provide a brief example of a deck selection object? I'm unsure if it implies a list of DeckFields members or something else. Thanks.

[codecov]

Codecov Report

Attention: Patch coverage is 30.88235% with 47 lines in your changes missing coverage. Please review.

Project coverage is 51.19%. Comparing base (2719b34) to head (fb58060).
Report is 1 commits behind head on master.

Files	Patch %	Lines
flytekit/core/base_task.py	28.00%	16 Missing and 2 partials ⚠️
flytekit/deck/deck.py	41.66%	14 Missing ⚠️
flytekit/core/python_function_task.py	0.00%	9 Missing ⚠️
flytekit/core/context_manager.py	25.00%	6 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (2719b34) and HEAD (fb58060). Click for more details.

HEAD has 13 uploads less than BASE

Flag	BASE (2719b34)	HEAD (fb58060)
	15	2

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #2314       +/-   ##
===========================================
- Coverage   77.65%   51.19%   -26.47%     
===========================================
  Files         236      186       -50     
  Lines       20789    18855     -1934     
  Branches     3661     3690       +29     
===========================================
- Hits        16144     9652     -6492     
- Misses       4017     8713     +4696     
+ Partials      628      490      -138     

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

[pingsutw]

cc @thomasjpfan

[novahow]

Should we define it as a List[DeckFields] instead?

[thomasjpfan]

I am +0 on using enums if we reimport it in flytekit.deck. Specifically:

from flytekit.deck import PrebuiltDeck

@task(decks=[PrebuiltDeck.SourceCode, PrebuiltDeck.Inputs], enable_deck=True)

Enums is nice from a programming point of view, but they can make the feature harder to discover. For example, using strings means no additional imports:

@task(decks=["source_code", "inputs"], enable_deck=True)

@novahow From a user point of view, do you have a preference?

[novahow]

@thomasjpfan Yeah I think using strings is fine.

[pingsutw]

I prefer enum. how do users know what other decks we have? Using string is also error-prone. what if people use sourceCode or Input

[thomasjpfan]

We list the strings out in the docstring and remember to add new ones to the list. If someone uses sourceCode or Input, then we raise an error.

In the PyData world, most APIs are strings instead of enums. For example, SciPy's minimize) function uses strings for method. The newly release torch.comple uses strings for backend and mode.

Personally, I prefer enums too, but I think most users in the Python data space are more familiar with strings.

[thomasjpfan]

Sure. Additionally, could you provide a brief example of a deck selection object? I'm unsure if it implies a list of DeckFields members or something else. Thanks.

I think the intention is to have all the decks be explicit. For example, this will only show the "source code" deck and not the input and output:

@task(decks=("source_code",), enable_deck=True)

If one wants to show the inputs and outputs, then they need to be explicit:

@task(decks=("source_code", "inputs", "outputs"), enable_deck=True)

In the examples above the TimelineDeck is not shown because it was not explicitly shown.

The other question is what the default should be. Personally, I want the default decks to be ("source_code",), because I think seeing the source code is generally useful. (The default is a tuple, so it is not mutable.) With this proposal, if enable_deck is false, then decks is ignored.

[thomasjpfan]

I am +0 on using enums if we reimport it in flytekit.deck. Specifically:

from flytekit.deck import PrebuiltDeck

@task(decks=[PrebuiltDeck.SourceCode, PrebuiltDeck.Inputs], enable_deck=True)

Enums is nice from a programming point of view, but they can make the feature harder to discover. For example, using strings means no additional imports:

@task(decks=["source_code", "inputs"], enable_deck=True)

@novahow From a user point of view, do you have a preference?

[novahow]

@thomasjpfan. Thanks for the comment.
So when

1. enable_deck=True, ignore decks and output all
2. enable_deck=None, use decks
3. enable_deck=False, ignore decks

Are these the expected behavior? Thanks

[thomasjpfan]

enable_deck=True, ignore decks and output all

I want the default decks to be ("source_code", "python_dependencies"), so that when enable_deck=True and decks is not passed in, then the source code and python dependencies are shown. I have not found many use case for the "Inputs" and "Outputs" deck, and the TimelineDeck requires a plugin, so I rather not show it by default.

enable_deck=None, use decks
enable_deck=False, ignore decks

In these cases, I prefer not to not to show any decks. I like having an explicit enable_deck=True to mean "show decks" and anything else means there is no decks.

[thomasjpfan]

Thanks for the updates! Overall, I like the approach.

[thomasjpfan]

The CI errors look related.

[thomasjpfan]

Thanks for the update!

[thomasjpfan]

Last bit about the enum API, otherwise this looks good to go!

[pingsutw]

I made some changes.

• Render time table without installing flytekitplugins-deck-standard
• Add timeline chart to the deck if flytekitplugins-deck-standard is installed