Introducing whylogs integration to flytekit #1104

murilommen · 2022-07-14T19:43:51Z

TL;DR

Whylogs plugin integration

Type

Bug Fix
Feature
Plugin

Are all requirements met?

Complete description

It creates a schema for "transforming" whylogs' DatasetProfileView objects to and from the FlyteSchema. This PR also creates two renderers, one for whylogs' Constraints report and the other for the SummaryDrift Report.

Tracking Issue

NA

Follow-up issue

NA

NOTE: This PR solves for #1100 . Had to make some fixes for wrongly signed-off commits

* Details: It creates a schema for "transforming" whylogs' DatasetProfileView objects to and from the FlyteSchema. This PR also creates two renderers with FlyteDeck, one for the Constraints report and the other for the SummaryDriftReport. Signed-off-by: murilommen <[email protected]>

changes: breaking down single test_schema.py unit test into test_schema + test_renderer, each with their own concerns Signed-off-by: murilommen <[email protected]>

details: removing extra lines later caught by flake8 Signed-off-by: murilommen <[email protected]>

eapolinario

This looks good, let's just ensure the tests you added run in CI. In order to do that, can you add the new plugin to this list?

Signed-off-by: murilommen <[email protected]>

plugins/flytekit-whylogs/setup.py

wild-endeavor · 2022-07-15T17:49:39Z

this looks good. do you think you'll be able to add an example to https://github.com/flyteorg/flytesnacks/tree/master/cookbook/integrations/flytekit_plugins as well please?

codecov · 2022-07-15T17:55:11Z

Codecov Report

Merging #1104 (e7763a4) into master (58be244) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1104      +/-   ##
==========================================
+ Coverage   86.91%   86.93%   +0.01%     
==========================================
  Files         275      275              
  Lines       25435    25448      +13     
  Branches     2506     2862     +356     
==========================================
+ Hits        22108    22123      +15     
+ Misses       2851     2847       -4     
- Partials      476      478       +2

Impacted Files	Coverage Δ
flytekit/tools/script_mode.py	`73.52% <0.00%> (-0.39%)`	⬇️
flytekit/interfaces/random.py	`100.00% <0.00%> (ø)`
flytekit/clis/flyte_cli/main.py	`44.34% <0.00%> (ø)`
flytekit/extras/persistence/http.py	`69.38% <0.00%> (ø)`
flytekit/types/directory/types.py	`94.16% <0.00%> (+0.04%)`	⬆️
flytekit/types/file/file.py	`92.36% <0.00%> (+0.05%)`	⬆️
flytekit/types/schema/types.py	`76.73% <0.00%> (+0.09%)`	⬆️
flytekit/models/security.py	`87.17% <0.00%> (+0.33%)`	⬆️
flytekit/core/promise.py	`79.09% <0.00%> (+0.37%)`	⬆️
flytekit/configuration/internal.py	`95.83% <0.00%> (+0.52%)`	⬆️

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 58be244...e7763a4. Read the comment docs.

Signed-off-by: murilommen <[email protected]>

eapolinario · 2022-07-15T20:29:41Z

It looks like version 1.0.6 of whylogs is not available in pypi. I'm seeing the same error in a local venv:

❯ pip install whylogs==1.0.6
ERROR: Could not find a version that satisfies the requirement whylogs==1.0.6 (from versions: 0.0.2b10, 0.0.2b13, 0.1.0, 0.1.1b0, 0.1.1, 0.1.2b0, 0.1.2, 0.1.3, 0.1.4, 0.1.5, 0.1.6.dev0, 0.1.6, 0.1.8, 0.1.10, 0.1.13.dev3, 0.1.13.dev4, 0.1.13.dev5, 0.1.13.dev6, 0.1.13.dev7, 0.1.13.dev8, 0.1.13, 0.2.0.dev0, 0.3.0, 0.3.1.dev0, 0.3.1.dev1, 0.3.1.dev2, 0.3.1.dev3, 0.3.1, 0.3.2, 0.3.3.dev0, 0.3.3.dev1, 0.3.3.dev2, 0.3.3.dev3, 0.3.3.dev4, 0.3.3.dev5, 0.3.3.dev6, 0.3.3.dev7, 0.3.4.dev1, 0.4.0.dev0, 0.4.1.dev0, 0.4.2.dev0, 0.4.3.dev0, 0.4.4.dev0, 0.4.5.dev0, 0.6.20.dev0, 0.6.20.dev1, 0.6.21, 0.6.22, 1.0.0.dev0, 1.0.0rc1, 1.0.0rc2, 1.0.0rc3, 1.0.0rc4, 1.0.0)
ERROR: No matching distribution found for whylogs==1.0.6

eapolinario · 2022-07-15T20:49:43Z

It looks like version 1.0.6 of whylogs is not available in pypi. I'm seeing the same error in a local venv:

❯ pip install whylogs==1.0.6
ERROR: Could not find a version that satisfies the requirement whylogs==1.0.6 (from versions: 0.0.2b10, 0.0.2b13, 0.1.0, 0.1.1b0, 0.1.1, 0.1.2b0, 0.1.2, 0.1.3, 0.1.4, 0.1.5, 0.1.6.dev0, 0.1.6, 0.1.8, 0.1.10, 0.1.13.dev3, 0.1.13.dev4, 0.1.13.dev5, 0.1.13.dev6, 0.1.13.dev7, 0.1.13.dev8, 0.1.13, 0.2.0.dev0, 0.3.0, 0.3.1.dev0, 0.3.1.dev1, 0.3.1.dev2, 0.3.1.dev3, 0.3.1, 0.3.2, 0.3.3.dev0, 0.3.3.dev1, 0.3.3.dev2, 0.3.3.dev3, 0.3.3.dev4, 0.3.3.dev5, 0.3.3.dev6, 0.3.3.dev7, 0.3.4.dev1, 0.4.0.dev0, 0.4.1.dev0, 0.4.2.dev0, 0.4.3.dev0, 0.4.4.dev0, 0.4.5.dev0, 0.6.20.dev0, 0.6.20.dev1, 0.6.21, 0.6.22, 1.0.0.dev0, 1.0.0rc1, 1.0.0rc2, 1.0.0rc3, 1.0.0rc4, 1.0.0)
ERROR: No matching distribution found for whylogs==1.0.6

We talked offline and this has to do with whylogs only supporting python versions < 3.10. Update coming shortly.

Signed-off-by: murilommen <[email protected]>

details: whylogs require protobuf > 3.15, but flyteidl can't handle protobuf < 4.0, so adding it as a restriction to setup.py Signed-off-by: murilommen <[email protected]>

eapolinario

Make sure to run make fmt from the root of the repo.

plugins/flytekit-whylogs/setup.py

details: ran black command on the plugin implementation and caught something with black Signed-off-by: murilommen <[email protected]>

plugins/flytekit-whylogs/setup.py

Signed-off-by: murilommen <[email protected]>

welcome · 2022-07-18T19:01:01Z

Congrats on merging your first pull request! 🎉

* Introducing whylogs integration to flytekit * Details: It creates a schema for "transforming" whylogs' DatasetProfileView objects to and from the FlyteSchema. This PR also creates two renderers with FlyteDeck, one for the Constraints report and the other for the SummaryDriftReport. Signed-off-by: murilommen <[email protected]> * refactoring tests changes: breaking down single test_schema.py unit test into test_schema + test_renderer, each with their own concerns Signed-off-by: murilommen <[email protected]> * styleguide fixes details: removing extra lines later caught by flake8 Signed-off-by: murilommen <[email protected]> * add plugin to ci checks Signed-off-by: murilommen <[email protected]> * adding requirements files + lint fixes Signed-off-by: murilommen <[email protected]> * removing entrypoints from setup.py Signed-off-by: murilommen <[email protected]> * adding a python 3.10 build restriction Signed-off-by: murilommen <[email protected]> * fixing protobuf's version to < 4 details: whylogs require protobuf > 3.15, but flyteidl can't handle protobuf < 4.0, so adding it as a restriction to setup.py Signed-off-by: murilommen <[email protected]> * lint setup.py details: ran black command on the plugin implementation and caught something with black Signed-off-by: murilommen <[email protected]> * fixing comma on setup.py Signed-off-by: murilommen <[email protected]>

murilommen requested review from wild-endeavor, kumare3, eapolinario and pingsutw as code owners July 14, 2022 19:43

murilommen added 2 commits July 14, 2022 19:44

refactoring tests

28c9396

changes: breaking down single test_schema.py unit test into test_schema + test_renderer, each with their own concerns Signed-off-by: murilommen <[email protected]>

styleguide fixes

6643089

details: removing extra lines later caught by flake8 Signed-off-by: murilommen <[email protected]>

eapolinario reviewed Jul 15, 2022

View reviewed changes

add plugin to ci checks

5ee468c

Signed-off-by: murilommen <[email protected]>

wild-endeavor reviewed Jul 15, 2022

View reviewed changes

plugins/flytekit-whylogs/setup.py Outdated Show resolved Hide resolved

murilommen added 2 commits July 15, 2022 15:36

adding requirements files + lint fixes

96db600

Signed-off-by: murilommen <[email protected]>

removing entrypoints from setup.py

1475a85

Signed-off-by: murilommen <[email protected]>

murilommen force-pushed the murilommen/whylogs-plugin branch from e4104bd to 1475a85 Compare July 15, 2022 20:04

murilommen added 2 commits July 15, 2022 17:58

adding a python 3.10 build restriction

93e6fb3

Signed-off-by: murilommen <[email protected]>

fixing protobuf's version to < 4

2215196

details: whylogs require protobuf > 3.15, but flyteidl can't handle protobuf < 4.0, so adding it as a restriction to setup.py Signed-off-by: murilommen <[email protected]>

eapolinario reviewed Jul 15, 2022

View reviewed changes

plugins/flytekit-whylogs/setup.py Outdated Show resolved Hide resolved

plugins/flytekit-whylogs/setup.py Outdated Show resolved Hide resolved

lint setup.py

e908b2f

details: ran black command on the plugin implementation and caught something with black Signed-off-by: murilommen <[email protected]>

eapolinario reviewed Jul 18, 2022

View reviewed changes

plugins/flytekit-whylogs/setup.py Outdated Show resolved Hide resolved

fixing comma on setup.py

e7763a4

Signed-off-by: murilommen <[email protected]>

murilommen force-pushed the murilommen/whylogs-plugin branch from b62c81d to e7763a4 Compare July 18, 2022 16:21

eapolinario approved these changes Jul 18, 2022

View reviewed changes

eapolinario merged commit c5a9468 into flyteorg:master Jul 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing whylogs integration to flytekit #1104

Introducing whylogs integration to flytekit #1104

murilommen commented Jul 14, 2022

eapolinario left a comment

wild-endeavor commented Jul 15, 2022

codecov bot commented Jul 15, 2022 •

edited

Loading

eapolinario commented Jul 15, 2022

eapolinario commented Jul 15, 2022

eapolinario left a comment

welcome bot commented Jul 18, 2022

Introducing whylogs integration to flytekit #1104

Introducing whylogs integration to flytekit #1104

Conversation

murilommen commented Jul 14, 2022

TL;DR

Type

Are all requirements met?

Complete description

Tracking Issue

Follow-up issue

eapolinario left a comment

Choose a reason for hiding this comment

wild-endeavor commented Jul 15, 2022

codecov bot commented Jul 15, 2022 • edited Loading

Codecov Report

eapolinario commented Jul 15, 2022

eapolinario commented Jul 15, 2022

eapolinario left a comment

Choose a reason for hiding this comment

welcome bot commented Jul 18, 2022

codecov bot commented Jul 15, 2022 •

edited

Loading