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.

Sign up for GitHub

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

Jump to bottom

Moves scaling out of experimental #286

Merged
merged 6 commits into from
Aug 24, 2023
Merged

Moves scaling out of experimental #286

merged 6 commits into from
Aug 24, 2023

Conversation

elijahbenizzy
Copy link
Collaborator

[Short description explaining the high-level reason for the pull request]

Changes

How I tested this

Notes

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch 2 times, most recently from a749b00 to 2fa412c Compare August 22, 2023 21:51
@elijahbenizzy elijahbenizzy changed the title WIP Moves scaling out of experimental Aug 22, 2023
@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch 5 times, most recently from 14c5519 to c2720cd Compare August 22, 2023 22:31
@elijahbenizzy elijahbenizzy requested a review from skrawcz August 22, 2023 22:33
@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch 3 times, most recently from 09a20b6 to f8241da Compare August 23, 2023 05:03
@elijahbenizzy
Moves all scaling related constructs out of experimental
4deb3da 
- h_spark
- h_dask
- h_ray

All move to plugins. We preserve the name `h_...` to avoid duplicate
imports from the library themselves. Note that we also deprecate the
idea of having "_implementations". People will be importing these, so we
want them name to be easy to refer to/remember. This is why
polars_implementations is deprecated, and we instead use h_polars.

We leave in references to all previously released constructs (note,
the new pyspark API has not been released so its OK to remove that from
the experimental.h_spark file).
@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch 5 times, most recently from 6d30b24 to 8236b47 Compare August 23, 2023 05:18
skrawcz
skrawcz reviewed Aug 23, 2023
View reviewed changes
hamilton/registry.py Outdated Show resolved Hide resolved
@skrawcz
Copy link
Collaborator

skrawcz commented Aug 23, 2023

Need a README somewhere too (maybe in plugins) to explain how to add/adjust and name files.

skrawcz
skrawcz reviewed Aug 23, 2023
View reviewed changes
Copy link
Collaborator

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

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

Plugins will get large. We also don't want very large files either.

So the evolution should be into packages, right?
h_spark.py -->

  • init.py
  • ... modules organized however we want

And then similarly for *_extensions.py...?

@elijahbenizzy
Copy link
Collaborator Author

Plugins will get large. We also don't want very large files either.

So the evolution should be into packages, right? h_spark.py -->

  • init.py
  • ... modules organized however we want

And then similarly for *_extensions.py...?

Yeah, I think that'll be the best way for it to evolve. Should be able to do it and keep compatible references later on, no need to break into packages yet.

@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch from 8236b47 to a445aa3 Compare August 23, 2023 19:23
@elijahbenizzy
Removes strict extension registration
514ef64 
This is for getting it to work with pyspark. While it does have natural
column/dataframe objects, these are not usable in the same way. Thus we
don't want to register the same set of constructs as we would, say, with
polars or pandas.

We allow plugins to opt out by specifying COLUMN_FRIENDLY_DF_TYPE =
False, which defaults to true.
@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch from a445aa3 to 36b7414 Compare August 23, 2023 19:29
@elijahbenizzy
Fixes pyspark depenencies in install target
f190f6e 
Pyspark has a bunch of hidden depedencies -- these are solved by the sql
and connect targets
@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch from 36b7414 to f190f6e Compare August 23, 2023 20:30
@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch from 05ce983 to 18e7d65 Compare August 23, 2023 21:50
@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch 2 times, most recently from e460bde to 48084cc Compare August 24, 2023 00:12
@elijahbenizzy elijahbenizzy force-pushed the scaling-out-of-experimental branch from 48084cc to 0bcbcec Compare August 24, 2023 00:13
skrawcz
skrawcz approved these changes Aug 24, 2023
View reviewed changes
Copy link
Collaborator

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

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

I think things work.

@elijahbenizzy elijahbenizzy merged commit bc9f2b9 into main Aug 24, 2023
@elijahbenizzy elijahbenizzy deleted the scaling-out-of-experimental branch August 24, 2023 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skrawcz skrawcz skrawcz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@elijahbenizzy @skrawcz