nbformat icon indicating copy to clipboard operation
nbformat copied to clipboard

outputs_hidden vs collapsed metadata

Open jasongrout opened this issue 6 years ago • 17 comments

I'm trying to implement the appropriate semantics for collapsed/jupyter.outputs_hidden metadata in JupyterLab.

The format docs seem ambiguous to me about the difference between the cell collapsed vs jupyter.outputs_hidden metadata.

In the nbformat docs, we have the following descriptions:

collapsed: Whether the cell’s output container should be collapsed jupyter.outputs_hidden: Whether the cell’s outputs should be shown

In the discussion introducing outputs_hidden (https://github.com/jupyter/nbformat/pull/94), @ellisonbg mentions the confusion, and @rgbkrk and @damianavila mention in the next two comments that they view collapsed as essentially setting a small fixed height for the output, while outputs_hidden is completely hiding the outputs. (To me, the separate scrolled metadata is about setting a height for the outputs.)

How has this played out in practice over the last several years since the outputs_hidden field was defined? Classic notebook still only sets collapsed, and setting it hides the outputs, not just shortening them (scrolled metadata shortens the output region). As far as I can tell, the classic notebook doesn't recognize the jupyter.outputs_hidden metadata.

Does anyone else respect the jupyter.outputs_hidden metadata, or treat it differently from the collapsed metadata?

CC @rgbkrk, @mpacer, @ellisonbg

jasongrout avatar Mar 26 '19 17:03 jasongrout

Not sure if you have seen this already, but there was a discussion on this topic when I was implementing storing this state in Jupyterlab: https://github.com/jupyterlab/jupyterlab/pull/3981#issuecomment-391139167. What I implemented only supports collapsed, not outputs_hidden, although from the discussion it seems like the plan was to possibly support both.

saulshanabrook avatar Mar 26 '19 19:03 saulshanabrook

You actually did implement outputs_hidden too:

https://github.com/jupyterlab/jupyterlab/pull/3981/files#diff-901b6622e3beb3db23ab3289d24f5583R609

https://github.com/jupyterlab/jupyterlab/pull/3981/files#diff-fec8375f9426c263bcd547883abe072eR1112

https://github.com/jupyterlab/jupyterlab/pull/3981/files#diff-61a6c4b1334045caf8dd59d42319e1d8R400

jasongrout avatar Mar 26 '19 19:03 jasongrout

Tangentially related - the source_hidden field specified in the same nbformat PR was implemented in papermill in https://github.com/nteract/papermill/pull/135, but not the outputs_hidden field.

jasongrout avatar Mar 26 '19 19:03 jasongrout

CC also @mseal, who was involved in the source_hidden implementation in papermill.

jasongrout avatar Mar 26 '19 19:03 jasongrout

@rgbkrk mentioned in https://github.com/nteract/papermill/pull/135#issuecomment-391150091 that the plan was for nteract to support outputs_hidden.

jasongrout avatar Mar 26 '19 20:03 jasongrout

If it helps, https://github.com/jupyter/notebook/issues/534 is a thread that needs to be reignited to add support for the source_hidden and outputs_hidden in classic (I get this requests a lot through papermill to add support to classic UI). I don't have a strong opinion on the fields, beyond that we have a consistent contract set in nbformat and conform the various interfaces to it. I've found *_hidden in the cell and notebook level to be the obvious for users to try and use, and discover doesn't work in all places. I'd be ok with conforming to that naming convention moving forward, though I have little stake or requirements to maintain collapsed so I'll keep to supporting what we finalize on in the schema.

MSeal avatar Mar 26 '19 20:03 MSeal

FYI, because of the apparent conflict between collapsed and jupyter.outputs_hidden, what I implemented in my PR to JupyterLab is that it only reads the collapsed field and ignores the jupyter.outputs_hidden field, but it will write to both collapsed and jupyter.outputs_hidden. I did this to maintain backwards compatibility with the classic notebook.

I'm tempted to make it just ignore jupyter.outputs_hidden altogether, unless other tools are using that field, or unless there actually is a semantic difference between the two fields.

jasongrout avatar Mar 27 '19 04:03 jasongrout

I decided to go ahead and not implement jupyter.outputs_hidden in my current PR to JupyterLab, given the redundancy in the format spec. I think it's better to ignore the field than to possibly overwrite a conflict between it and collapsed without recognizing the conflict. See https://github.com/jupyterlab/jupyterlab/pull/5968#issuecomment-477170937.

jasongrout avatar Mar 27 '19 14:03 jasongrout

Maybe we should get weigh in from others before finalizing this in a release? I'd prefer we're all consistent and have the opinion hit nbformat close there-after. If there's not consistent feedback today we should probably support one as a fallback name for the other until it's unified in nbformat. I would note that jupyter.outputs_hidden is a much clearer name than collapsed if we're future looking for format adoption.

MSeal avatar Mar 27 '19 15:03 MSeal

Chiming in with my two cents: the description for collapsed defines it as a way to show and expand a cell, where as outputs_hidden and source_hidden are for the individual outputs/inputs within a cell. For example, have collapsed set to true produces the same effect as having outputs_hidden and source_hidden set to true.

Similar to @MSeal's perspective, I lean towards standardizing formally on output_hidden and input_hidden. The terms are precise, clear, and granular enough to be spec-worthy.

captainsafia avatar Mar 27 '19 17:03 captainsafia

@captainsafia, thanks. In the nbformat docs, it explicitly says collapsed is "Whether the cell’s output container should be collapsed" - to me, that's not saying anything about the input/source.

I don't think we can ignore collapsed in nbformat 4.x, since the classic notebook already has a behavior for collapsed, and possibly millions of notebooks out there use that interpretation. I don't think we can change that behavior in a minor version rev of nbformat. (I don't think anyone is suggesting that we ignore collapsed, but just wanted to be clear...)

@Mseal, you suggested one to be a fallback of the other. Which one do you suggest be the primary source of truth? For pragmatic reasons, it makes sense to me for applications to write to both jupyter.outputs_hidden and collapsed (so the output collapsing happens in current/older versions of classic notebook). But if you're doing that, then you might as well just write to collapsed and ignore jupyter.outputs_hidden.

Another thing I realized: though not explicitly defined in the standard, each of these keys pragmatically has a default which is used when there is not a value for the field. These defaults probably ought to be explicitly defined in the standard. (For example, collapsed I think is generally understood to default to false).

For nbformat 5, I totally agree that we can resolve the confusion and have the elegance of the namespace by dropping collapsed.

jasongrout avatar Mar 28 '19 15:03 jasongrout

Yes, my comment was more suggesting a naming opinion for v5 if we want to unify more there for the future.

I see what you're saying with conforming to v4 naming, however along those line of thinking code cells already have jupyter.source_hidden, jupyter.outputs_hidden and collapsed defined in the spec: https://github.com/jupyter/nbformat/blob/master/nbformat/v4/nbformat.v4.schema.json#L199-L211. I don't think we could remove one in v4 but if you're conforming to the spec, jlab should respect all 3 fields for v4 notebooks. If the implementation for output removal is the same for the two overlapping fields in jlab, then perhaps make cells outputs visible by basic boolean logic of the the union of the signals visible = (jupyter.outputs_hidden == null || jupyter.outputs_hidden) && (collapsed == null || collapsed). By the spec argument classic is also not fully supporting the v4 format today either.

I like the default value in the spec idea. We should absolutely add that. @rgbkrk is unfortunately on paternity leave, but I believe his input for v5 changes would be best to get before we commit to anything there.

MSeal avatar Mar 28 '19 16:03 MSeal

@jasongrout Oh boy! Looks like the docs and the schema disagree. In the schema, it refers to the entire cell. Assuming the docs are the source of truth, I've submitted https://github.com/jupyter/nbformat/pull/141 to correct this.

Agreed on dropping collapsed in nbformat 5.

captainsafia avatar Mar 28 '19 21:03 captainsafia

@captainsafia - oh, great catch! In cases where there is ambiguity between docs and spec, I think the classic notebook behavior should be the source of truth, since I think that is still how the vast majority of users have interacted with the notebook format (if not currently, then certainly in the past...)

jasongrout avatar Mar 29 '19 21:03 jasongrout

however along those line of thinking code cells already have jupyter.source_hidden, jupyter.outputs_hidden and collapsed defined in the spec: https://github.com/jupyter/nbformat/blob/master/nbformat/v4/nbformat.v4.schema.json#L199-L211

Yes, technically, since 2017. Though I think this conversation was started partly because we realized that most frontends we know about were actually only implementing nbformat 4.2, so I feel like we have a bit more leeway in how we use the new 4.4 fields than the collapsed field from forever ago.

By the spec argument classic is also not fully supporting the v4 format today either.

That's why I think I want to still write to collapsed - to support users that want to use the classic notebook.

perhaps make cells outputs visible by basic boolean logic of the the union of the signals

Maybe what I'll do is essentially:

if (metadata.jupyter.outputs_hidden !== undefined) {
  if (metadata.collapsed === undefined) {
    metadata.collapsed = metadata.jupyter.outputs_hidden;
  }
  delete metadata.jupyter.outputs_hidden;
}

That way collapsed has precedence, but if jupyter.outputs_hidden is defined and collapsed isn't, I can still render appropriately, and as a bonus the new metadata is recognized in the classic notebook.

jasongrout avatar Mar 29 '19 22:03 jasongrout

That way collapsed has precedence, but if jupyter.outputs_hidden is defined and collapsed isn't, I can still render appropriately, and as a bonus the new metadata is recognized in the classic notebook.

Sure. That'll work well enough. We can talk about v5 field support separately.

MSeal avatar Mar 29 '19 22:03 MSeal

Actually, I changed my mind again. https://github.com/jasongrout/jupyterlab/commit/36dfc3e5bf57e623b5206103b70204f5a363f426 in JupyterLab ensures that collapsed and jupyter.outputs_hidden are always the same thing, with preference to collapsed if it is defined and there is a conflict.

jasongrout avatar Apr 01 '19 21:04 jasongrout