emdgroup / foundry-dev-tools

Foundry DevTools
https://emdgroup.github.io/foundry-dev-tools/
Apache License 2.0
115 stars 23 forks source link

Add preview support for Lightweight transforms #47

Closed schmelczer closed 9 months ago

schmelczer commented 10 months ago

Summary

Address https://github.com/emdgroup/foundry-dev-tools/issues/45

Add support for running local preview for @lightweight and @transform_polars Transforms which were recently added to the Foundry Transforms library.

Limitations

Checklist

CLAassistant commented 10 months ago

CLA assistant check
All committers have signed the CLA.

schmelczer commented 10 months ago

I'm coordinating with legal regarding the CLA

jonas-w commented 10 months ago

@schmelczer thank you so much for this great contribution, I'll take a closer look at it tomorrow!

schmelczer commented 9 months ago

We're still actively looking into the CLA

schmelczer commented 9 months ago

The CLA has been cleared, so I've signed it! Feel free to take a look at the PR @jonas-w, I'm happy to discuss the details, especially if there're any counter-arguments against the lazy dataset loading

nicornk commented 9 months ago

Thanks for implementing the laziness in the decorators, something that was on our long list as well.

@schmelczer 🙏 thanks for the contribution.

schmelczer commented 9 months ago

Thanks for giving this a proper review, I've addressed your comments!

I've done some further testing with schema-less datasets for both lightweight & regular transforms and preview seems to be working as expected! As for the logging, there isn't a trivial way of implementing this (we could look at the JobSpec though) but the foundry telemetry service update is just around the corner which will work with both Spark and Lightweight transforms!

nicornk commented 9 months ago

lgtm

jonas-w commented 9 months ago

@schmelczer would it be possible to remove this merge commit d111123 (#47) and instead do a rebase? This way the history commit history would stay a bit cleaner after we merge it into main.

Otherwise it looks good to me!

schmelczer commented 9 months ago

Sure, I've dropped the merge commit @jonas-w!

jonas-w commented 9 months ago

Should be released by now https://pypi.org/project/foundry-dev-tools/

Thanks again @schmelczer!