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

The default behaviour of put is not sane enough for unknown object types #822

Open
augustebaum opened this issue Nov 27, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@augustebaum
Copy link
Contributor

augustebaum commented Nov 27, 2024

Is your feature request related to a problem? Please describe.

The current default when an unknown object type is put is to raise an ItemTypeError.

The user has two solutions if they want to store their object in skore:

  • Use MediaItem, i.e. rather than:
project.put("key", my_obj)

write for example

project.put_item("key", MediaItem.factory(my_obj_as_bytes_or_str, media_type="text/html"))

This means the user has the responsibility of passing the media type and to convert their object to bytes or string form, which they are unlikely to do in practice.

Both of these solutions lower the usability of skore.

Describe the solution you'd like

We should support put for any object that has a _repr_html_ method, and when this is the case we do the conversion to MediaItem ourselves.

In the future we can also consider other media types by detecting other _repr_xxx_ methods, e.g. by looking first for _repr_html_, then _repr_svg_, then _repr_png_, etc. We might also make use of _repr_mimebundle_.

Describe alternatives you've considered, if relevant

Use pickle (not environment-dependent, but could be more useful for users)

Additional context

This issue is derived from #810 (comment)

@augustebaum augustebaum added enhancement New feature or request needs-triage This has been recently submitted and needs attention labels Nov 27, 2024
@MarieS-WiMLDS MarieS-WiMLDS removed the needs-triage This has been recently submitted and needs attention label Dec 10, 2024
@augustebaum
Copy link
Contributor Author

@MarieS-WiMLDS This doesn't seem ready to me

@MarieS-WiMLDS
Copy link
Contributor

MarieS-WiMLDS commented Dec 12, 2024

You describe the solution you want, so it seems ready to me. what is missing?

@rouk1
Copy link
Contributor

rouk1 commented Dec 13, 2024

I vote that we do this. This a refactor as it means that we will have to write a presenter for each Item type and remove the serialization related stuff from the API routes. This is a first step to have our widgets usable in notebook. See related PR #926 .

@augustebaum
Copy link
Contributor Author

@rouk1 I'm not sure I get what you mean. This issue simply says: "upon put, if the object matches none of our Item types, then try saving object._repr_html_()"

@augustebaum augustebaum self-assigned this Dec 16, 2024
@rouk1
Copy link
Contributor

rouk1 commented Dec 17, 2024

@rouk1 I'm not sure I get what you mean. This issue simply says: "upon put, if the object matches none of our Item types, then try saving object._repr_html_()"

You're right I did not red the issue summary correctly. My comment is irrelevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants