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

EnumType support in flytekit #509

Merged
merged 2 commits into from
Jun 15, 2021
Merged

EnumType support in flytekit #509

merged 2 commits into from
Jun 15, 2021

Conversation

kumare3
Copy link
Contributor

@kumare3 kumare3 commented Jun 8, 2021

TL;DR

This PR introduces Enum Types in flytekit (flyteidl will natively support it)

Example usage:

from enum import Enum

class Color(Enum):
   RED = "red"
   BLUE = "blue"
   GREEN = "green"

@task
def foo(c: Color) -> str:
   return c.value

UI/UX: Drop down. RED will be the default value
flytectl: enforce the value to be limited to one of the enum values

Signed-off-by: Ketan Umare [email protected]

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Design Doc

Tracking Issue

flyteorg/flyte#590

Example usage:

```python
from enum import Enum

class Color(Enum):
   RED = "red"
   BLUE = "blue"
   GREEN = "green"

@task
def foo(c: Color) -> str:
   return c.value
```

UI/UX: Drop down. RED will be the default value
flytectl: enforce the value to be limited to one of the enum values

Signed-off-by: Ketan Umare <[email protected]>
@codecov
Copy link

codecov bot commented Jun 8, 2021

Codecov Report

Merging #509 (4069aed) into master (62391ea) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #509      +/-   ##
==========================================
+ Coverage   85.28%   85.32%   +0.03%     
==========================================
  Files         369      369              
  Lines       28002    28077      +75     
  Branches     2270     2273       +3     
==========================================
+ Hits        23882    23957      +75     
  Misses       3501     3501              
  Partials      619      619              
Impacted Files Coverage Δ
flytekit/core/type_engine.py 85.99% <100.00%> (+0.57%) ⬆️
flytekit/models/core/types.py 100.00% <100.00%> (ø)
flytekit/models/types.py 96.72% <100.00%> (+0.11%) ⬆️
tests/flytekit/unit/core/test_type_engine.py 99.55% <100.00%> (+0.07%) ⬆️
tests/flytekit/unit/models/core/test_types.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62391ea...4069aed. Read the comment docs.

cosmicBboy
cosmicBboy previously approved these changes Jun 8, 2021
Walk the inheritance hierarchy of v and find a transformer that matches the first base class.
This is potentially non-deterministic - will depend on the registration pattern.

TODO lets make this deterministic by using an ordered dict
Copy link
Contributor

Choose a reason for hiding this comment

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

aren't python 3.6+ dicts ordered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohh really? let me check on that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cosmicBboy you are right, they are insertion order preserving as explained here, but not to be confused with ordered dicts. For our usecase we want OrderedDicts right? Insertion order depends on module load order, which can be arbitrary

wild-endeavor
wild-endeavor previously approved these changes Jun 9, 2021
@kumare3 kumare3 dismissed stale reviews from wild-endeavor and cosmicBboy via 4069aed June 11, 2021 23:59
@kumare3 kumare3 merged commit 74f063c into master Jun 15, 2021
EngHabu pushed a commit that referenced this pull request Jun 24, 2021
Example usage:

```python
from enum import Enum

class Color(Enum):
   RED = "red"
   BLUE = "blue"
   GREEN = "green"

@task
def foo(c: Color) -> str:
   return c.value
```

UI/UX: Drop down. RED will be the default value
flytectl: enforce the value to be limited to one of the enum values

Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
EngHabu pushed a commit that referenced this pull request Jun 24, 2021
Example usage:

```python
from enum import Enum

class Color(Enum):
   RED = "red"
   BLUE = "blue"
   GREEN = "green"

@task
def foo(c: Color) -> str:
   return c.value
```

UI/UX: Drop down. RED will be the default value
flytectl: enforce the value to be limited to one of the enum values

Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
EngHabu pushed a commit that referenced this pull request Jun 25, 2021
Example usage:

```python
from enum import Enum

class Color(Enum):
   RED = "red"
   BLUE = "blue"
   GREEN = "green"

@task
def foo(c: Color) -> str:
   return c.value
```

UI/UX: Drop down. RED will be the default value
flytectl: enforce the value to be limited to one of the enum values

Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
wild-endeavor pushed a commit that referenced this pull request Jun 30, 2021
Example usage:

```python
from enum import Enum

class Color(Enum):
   RED = "red"
   BLUE = "blue"
   GREEN = "green"

@task
def foo(c: Color) -> str:
   return c.value
```

UI/UX: Drop down. RED will be the default value
flytectl: enforce the value to be limited to one of the enum values

Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: wild-endeavor <[email protected]>
wild-endeavor pushed a commit that referenced this pull request Jun 30, 2021
Example usage:

```python
from enum import Enum

class Color(Enum):
   RED = "red"
   BLUE = "blue"
   GREEN = "green"

@task
def foo(c: Color) -> str:
   return c.value
```

UI/UX: Drop down. RED will be the default value
flytectl: enforce the value to be limited to one of the enum values

Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: wild-endeavor <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants