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

[Spike] Create a Proof of Concept for Type Hints for dataset previews #2090

Closed
1 task
SajidAlamQB opened this issue Sep 11, 2024 · 6 comments
Closed
1 task

Comments

@SajidAlamQB
Copy link
Contributor

Description

See here: #1847

By using a type dict or other typing tools, see if the IDE will be able to provide hints that lets users know about the required structure of the preview dictionary (e.g., index, columns, and data keys for TablePreview). Assess how effective the PoC is in helping users keep to the expected preview format and reducing the likelihood of errors due to incorrect return types or structures.

Checklist

  • Include labels so that we can categorise your feature request
@SajidAlamQB
Copy link
Contributor Author

SajidAlamQB commented Sep 11, 2024

Hey @astrojuanlu @rashidakanchwala,

I replaced the existing NewType definitions for TablePreview, ImagePreview, PlotlyPreview, and JSONPreview with TypedDict and TypeAlias. Specifically, TypedDict was used for TablePreview to explicitly give its expected structure (e.g., index, columns, and data).

See: kedro-org/kedro-plugins#831

Results:

  • Type Hinting in IDEs: I verified that the IDE successfully suggests the required keys (like index, columns, and data) when implementing the TablePreview return value within a preview() method. This would definitely help users in reducing potential errors when creating custom dataset previews.

Issue:

  • One issue I found was that TypeAlias is only fully supported from Python 3.10 onwards.
  • Kedro-plugins currently supports Python 3.9, we may need look at alternatives or extra steps for backward compatibility, maybe maintaining the NewType approach or using conditional imports depending on the Python version?

Let me know your thoughts.

Example type hint for ImagePreview:

image

Auto suggestion for TablePreview return:

image

@SajidAlamQB SajidAlamQB self-assigned this Sep 11, 2024
@rashidakanchwala
Copy link
Contributor

This is amazing! Thanks for the spike.

I had one question. In Juanlu's example he returned a PolarsTable which had a differnt format. He didn't type it out just returned df (which was columns:...) -- does it highlight then too, if this is wrong return type

@SajidAlamQB
Copy link
Contributor Author

SajidAlamQB commented Sep 11, 2024

Good point I just tested it out and unfortunately it doesn't highlight it, TypedDict seems to provide basic hints, it doesn't enforce the structure strictly.

image

@astrojuanlu
Copy link
Member

Great job @SajidAlamQB and good call @rashidakanchwala ! Sad that my original issue wouldn't have been caught by this.

It's still a small improvement in DX, so maybe it would be nice to have.

@astrojuanlu
Copy link
Member

One issue I found was that TypeAlias is only fully supported from Python 3.10 onwards.

And the small improvement in DX is probably not worth the effort of doing this in a way that it's compatible with Python 3.9...

@SajidAlamQB
Copy link
Contributor Author

Thanks for the feedback. Since the type hinting provides some improvement in developer experience but doesn't strictly enforce the structure and has compatibility issues with Python 3.9, we'll close this spike. For now, we'll keep the current approach and revisit in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants