flytekit icon indicating copy to clipboard operation
flytekit copied to clipboard

Fix return type in `PanderaTransformer::to_python_value()`

Open danieldanciu opened this issue 1 year ago • 2 comments

Return an object of the correct type in the to_python_value of the pandera type transformer. Currently, the returned type is pandas.core.frame.DataFrame, but the declared return type is pandera.typing.DataFrame. I suspect this is because instantiating pandera.DataFrameSchema returns the former rather than what one would normally expect, which is the latter (the pandera DataFrame).

Closes flyteorg/flyte##5369

Why are the changes needed?

Typeguard is unhappy about Flyte workflows that use Pandera, because the type transformer returns an object of the incorrect type.

What changes were proposed in this pull request?

Simply mark the return type as pandera.typing.DataFrame (which inherits from the currently returned type, so I don't expect any major side-effects).

How was this patch tested?

Created a test workflow, checked with typeguard and everything is now running.

Setup process

Not needed.

Check all the applicable boxes

  • [ ] I updated the documentation accordingly.
  • [ ] All new and existing tests passed. I haven't run any tests, as there aren't any directly related tests AFAICT. I am hoping the Gitlab CI/CD will run them for me.
  • [x] All commits are signed-off.

danieldanciu avatar May 15 '24 13:05 danieldanciu

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

welcome[bot] avatar May 15 '24 13:05 welcome[bot]

cc @cosmicBboy

pingsutw avatar May 28 '24 16:05 pingsutw

Thanks for the contribution @danieldanciu !

I made some changes to the plugin recently, namely renamed the modules and updated some functionality: https://github.com/flyteorg/flytekit/tree/master/plugins/flytekit-pandera/flytekitplugins/pandera

Do you mind applying the change to the latest master branch

cosmicBboy avatar Oct 31 '24 19:10 cosmicBboy