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

Should we convert uploaded file metadata into Python types? #2727

Closed
pbugnion opened this issue Jan 13, 2020 · 5 comments
Closed

Should we convert uploaded file metadata into Python types? #2727

pbugnion opened this issue Jan 13, 2020 · 5 comments
Assignees
Labels
backwards-incompatible resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Milestone

Comments

@pbugnion
Copy link
Member

Following PRs #2724 and #2666 , the FileUpload.value traitlet looks like:

[{'name': 'example.csv',
  'type': 'text/csv',
  'size': 2548,
  'lastModified': 1536574751152,
  'content': <memory at 0x11d51ab88>}]

It is defined as List(Dict()).

We could consider the following, in increasing order of complexity:

  1. change lastModified to be a datetime object. We should check browser compatibility on whether it's always expressed as a timestamp.
  2. define a namedtuple or a Python class that exposes the attributes, rather than a dict. This would make the attributes more discoverable.

notifying @jtpio , @jasongrout who have been working on this.

@jasongrout
Copy link
Member

define a namedtuple or a Python class that exposes the attributes, rather than a dict. This would make the attributes more discoverable.

Why not have both? Use a Bunch! :)

https://github.com/ipython/traitlets/blob/master/traitlets/utils/bunch.py

(That's what we use for the change argument in traitlets change notifications)

@jtpio
Copy link
Member

jtpio commented Jan 13, 2020

Both suggestions sound good, especially if we want to stay close to the contents api: https://jupyter-notebook.readthedocs.io/en/stable/extending/contents.html#filesystem-entities

@jasongrout jasongrout added this to the 8.0 milestone Jan 14, 2020
@vidartf
Copy link
Member

vidartf commented Jan 20, 2020

As an xref, this might tie in with the datetime serialization in #2715. There, it is serialized along the pattern of DatePicker, as a dict of year, month, day, etc. Maybe we should switch to an ISO timestamp everywhere instead?

@pbugnion
Copy link
Member Author

I'll try and address this issue.

@jasongrout
Copy link
Member

Fixed in #2767.

@lock lock bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backwards-incompatible resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

No branches or pull requests

4 participants