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

feat(weave): Make ref-getting more pleasant #3362

Merged
merged 31 commits into from
Jan 14, 2025
Merged

Conversation

andrewtruong
Copy link
Collaborator

@andrewtruong andrewtruong commented Jan 10, 2025

Resolves:

This PR:

  1. Adds a mechanism, @register_object, to register objects that can be converted from WeaveObject into their base python classes. This can be used both internally and by end-users.
  2. Adds a new classmethod from_uri to registered objects
  3. Improves ref(...).get() to return the actual python class instead of WeaveObject for registered objects
  4. Adds optional refs on all weave.Object which are initially unset, but can be set when the object is re-created from a ref

Now you can do:

ds = Dataset(rows=[...])
ref = weave.publish(ds)

ds2 = ref.get()  # returns a `Dataset` instead of `WeaveObject`
ds3 = Dataset.from_uri(ref.uri())  # returns a `Dataset`

@andrewtruong andrewtruong changed the title Make ref-get return actual object instead of WeaveObject where possible feat(weave): Make ref-get return actual object instead of WeaveObject where possible Jan 10, 2025
@circle-job-mirror
Copy link

circle-job-mirror bot commented Jan 10, 2025

@andrewtruong andrewtruong changed the title feat(weave): Make ref-get return actual object instead of WeaveObject where possible feat(weave): Make ref-getting more pleasant Jan 10, 2025
@andrewtruong andrewtruong marked this pull request as ready for review January 13, 2025 04:42
@andrewtruong andrewtruong requested a review from a team as a code owner January 13, 2025 04:42

res = cls.from_obj(obj)
if ref := getattr(obj, "ref", None):
res.ref = ref
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if we prefer this or to set obj.__dict__["ref"]

f"`{cls.__name__}` must implement `from_obj` to support deserialization from a URI."
)

import weave
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we just import the ref method?

@andrewtruong andrewtruong enabled auto-merge (squash) January 14, 2025 23:20
@andrewtruong andrewtruong merged commit 1730168 into master Jan 14, 2025
123 checks passed
@andrewtruong andrewtruong deleted the andrew/objectify branch January 14, 2025 23:31
@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants