Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Add an initial cell model #2

Closed
wants to merge 3 commits into from
Closed

Conversation

blink1073
Copy link
Member

/**
* Get namespaced metadata about the cell.
*/
getMetadata(namespace: string) : IObservableMap<string, JSONData>;

Choose a reason for hiding this comment

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

I think that JSONData has to be any?

Choose a reason for hiding this comment

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

aren't we getting rid of metadata on the view models anyway? (we got rid of it on the notebook view model).

Choose a reason for hiding this comment

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

Hmm, no I think it should be there still on both the cells and notebook.
Don't remember it going away. Views will definitely end up needing it.

On Tue, Nov 3, 2015 at 7:56 PM, S. Chris Colbert notifications@github.com
wrote:

In src/index.ts
#2 (comment):

+interface IBaseCellViewModel {
+

  • /**
  • * The type of cell.
  • */
  • cellType: CellType;
  • /**
  • * Tags applied to the cell.
  • */
  • tags?: IObservableList;
  • /**
  • * Get namespaced metadata about the cell.
  • */
  • getMetadata(namespace: string) : IObservableMap<string, JSONData>;

aren't we getting rid of metadata on the view models anyway? (we got rid
of it on the notebook view model).


Reply to this email directly or view it on GitHub
https://github.com/jupyter/jupyter-js-cells/pull/2/files#r43830187.

Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com

Choose a reason for hiding this comment

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

What will the views use it for, that shouldn't otherwise be an actual property on the view model?

Choose a reason for hiding this comment

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

The main place where cell metadata is edited is by the cell toolbars, which is entirely a View-level aspect of the cell. I think that all of our existing cell toolbars serve to simply edit that cell metadata. Because of this the cell toobars (which will probably be provided using extensions/extension points) should probably be passed the ViewModel of the cell.

The metadata shouldn't be a single property on the ViewModel as we decided that we want to:

  1. Force people to namespace their metadata
  2. Enable people watch signals on their particular namespace changing in bulk.

Choose a reason for hiding this comment

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

I'm not clear what will be edited by these toolbars, and why that data cant be formally typed, or at least exposed at a level below these view models. I think a concrete use case would help me understand here.

Choose a reason for hiding this comment

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

In principle each library that starts to use a particular namespace in the
metadata could formally type the objects that appear in their namespace.Two
examples that we have today:

  • Slide show. Various parts of our code base can present a slide show view
    of the notebook. The cell metadata is used to encode how each cell with
    function in that slide show. This metadata is edited by a UI provided by a
    cell toolbar UI.
  • nbgrader. nbgrader has a cell toolbar UI that allows an instructor to
    encode how a cell will function in an assignment (test, points, etc.). Here
    are the docs for that:

http://nbgrader.readthedocs.org/en/stable/user_guide/02_developing_assignments.html

  • You could also imagine that the dashboard layout could get written to the
    cell/notebook metadata so when I email you a notebook, you can immediately
    access my dashboard layout.

All of this "data" is truly "metadata" in that it looses its meaning unless
it is linked to the cell/notebook.

There are many other libraries that are using notebook/cell metadata in
other ways.

The problem is that all of the cell toolbar UIs will be provided by
separate npm packages/plugins that provide celltoolbarview extensions to
some extension point. At the time we compile the code for notebook/cells,
we have no idea what the structural form of that metadata will be at
runtime. If we could know that in advance, then yes, we could formally type
it all.

However, it very well may be that the celltoolbar extension authors will
actually formally type this data in their own code - they will know that
the data looks like at compile time when types matter. Thus as long as they
only read/write the metadata object in their own namespace, the compiler
can promise type safety. This would help these plugins formalize the
information being stored in these fields.

On Tue, Nov 3, 2015 at 8:27 PM, S. Chris Colbert notifications@github.com
wrote:

In src/index.ts
#2 (comment):

+interface IBaseCellViewModel {
+

  • /**
  • * The type of cell.
  • */
  • cellType: CellType;
  • /**
  • * Tags applied to the cell.
  • */
  • tags?: IObservableList;
  • /**
  • * Get namespaced metadata about the cell.
  • */
  • getMetadata(namespace: string) : IObservableMap<string, JSONData>;

I'm not clear what will be edited by these toolbars, and why that data
cant be formally typed, or at least exposed at a level below these view
models. I think a concrete use case would help me understand here.


Reply to this email directly or view it on GitHub
https://github.com/jupyter/jupyter-js-cells/pull/2/files#r43832237.

Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com

Choose a reason for hiding this comment

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

It's not the type safety i'm worried about here; it's whether the cell/notebook view model is the right place for it. Are we expecting all implementations of ICell to provide this generic (meta)data store? If so, are they free to implement it in-memory only?

Otherwise, the metadata could be accessible at a lower layer of the model hierarchy, and we can expose a way to get to access those models which will be available to the cell toolbar.

Choose a reason for hiding this comment

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

It's the type safety i'm worried about here. It's whether the
cell/notebook view model is the right place for it. Are we expecting
all implementations of ICell to provide this generic (meta)data store?
If so, are they free to implement it in-memory only?

Yes, I would expect all implementations to provide this. I don't see any
problem with in-memory implementations. The only constraint - and this is a
type question - is that at some point, this data will be passed to a
serialization interface, which will attempt to JSON serialize it. This is
where TypeScript's lack of a JSONableData type is limiting. That would be
the the most accurate, and fully typesafe approach for the values in the
store.

Otherwise, the metadata could be accessible at a lower layer of the model
hierarchy, and we can expose a way to get to access those models which will
be available to the cell toolbar.

That is an option, but I don't see that approach buying us any additional
type safety - it is just pushing it down one level.


Reply to this email directly or view it on GitHub
https://github.com/jupyter/jupyter-js-cells/pull/2/files#r43834178.

Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com

Choose a reason for hiding this comment

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

Again, this point is not about type-safety for me; it's about ease of implementing the interface and proper separation of concerns. The thing(s) that will be shared across plugin boundaries should be models, not view models (typically), and I can easily see this metadata living there.

Here's some discussion on modelling a JSON value type (if you were curious):
microsoft/TypeScript#1897

/**
* The type of cell.
*/
cellType: CellType;
Copy link
Member

Choose a reason for hiding this comment

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

cellType or just type? otherwise it will read cell.cellType which seem a bit redundant.

Choose a reason for hiding this comment

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

I just grabbed it from the existing code where is it cell_type...I think
that type is a bit vague...

On Tue, Nov 3, 2015 at 9:12 PM, Matthias Bussonnier <
notifications@github.com> wrote:

In src/index.ts
#2 (comment):

+interface ICellChangedArgs {

  • name: string,
  • oldValue: T;
  • newValue: T;
    +}

+/**

  • * The definition of a model object for a base cell.
  • */
    +interface IBaseCellViewModel {
    +
  • /**
  • * The type of cell.
  • */
  • cellType: CellType;

cellType or just type? otherwise it will read cell.cellType which seem a
bit redundant.


Reply to this email directly or view it on GitHub
https://github.com/jupyter/jupyter-js-cells/pull/2/files#r43835466.

Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com

Choose a reason for hiding this comment

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

+1 for type

Copy link
Member

Choose a reason for hiding this comment

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

I just grabbed it from the existing code where is it cell_type...I think
that type is a bit vague...

Yes, but cell_type is an historical artefact, like code_cells having source property, and markdown_cells having text property. I think the global consistency of naming across the project is important. For example Steams have names : if I had only to consider the JS API (not the msg spec), I would use type to be consistent.

@blink1073
Copy link
Member Author

Updated based on comments.

@jasongrout
Copy link
Member

This is now superseded by (and included in) #3.

@jasongrout jasongrout closed this Dec 11, 2015
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.

None yet

5 participants