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

Fixed AttributeError when getting slide_type #604

Closed
wants to merge 1 commit into from

Conversation

Happyholic1203
Copy link
Contributor

To reproduce this AttributeError:

  1. Run jupyterlab: jupyter lab
  2. Create an ipynb
  3. Edit a cell and choose Cell Tools from the leftmost panel
  4. Set the cell to slide type

Now, the metadata looks like this:

{
  "collapsed": true,
  "slideshow": "slide"
}

Now, it's obvious that the original code would raise an AttributeError.

I got this all the time when using jupyterlab.
Maybe this should be fixed in jupyterlab, but I think adding a safe guard can't be harmful.
After all, you never know maybe one day it can become "slideshow": None

@takluyver
Copy link
Member

Ping @damianavila for slideshow metadata coordination :-)

@damianavila
Copy link
Member

@Happyholic1203 @takluyver I think this should be fixed in JupyterLab itself... @Happyholic1203 did you open an issue about this in that repo?

@Happyholic1203
Copy link
Contributor Author

@damianavila
Just did. jupyterlab issue here.

IMHO, I think it's actually better to fix this problem in nbconvert as well.
These two are independent projects,
and you can't prevent people from messing with the metadata.

One might as well set slideshow to None,
and it's valid to edit the json freely in both jupyterlab, jupyter notebook.

The comment in the code clearly states:

        # Make sure every cell has a slide_type
        cell.metadata.slide_type = cell.metadata.get('slideshow', {}).get('slide_type', '-')

The code clearly fails to achieve the comment when slide_type is not a dict, which makes it really easy to break. =P

@mpacer
Copy link
Member

mpacer commented Jul 20, 2017

So I'm hesitant to remove this Error. Or rather we may want to instead of "fixing" this, raise an error that explains that the users' notebook's metadata seems to be incorrectly specified. Since that's really the problem, not that the attribute is missing.

@damianavila
Copy link
Member

raise an error that explains that the users' notebook's metadata seems to be incorrectly specified

Sounds like a good idea.

@SylvainCorlay
Copy link
Member

@Happyholic1203 I would be happy to include this fix (and the suggestions), but the things have moved around quite a bit since you opened your PR - and there are conflicts. Would you be ok with opening an updated PR to master?

@MSeal
Copy link
Contributor

MSeal commented Jul 22, 2020

Closing as #1329 has been updated for 6.0 alpha

@MSeal MSeal closed this Jul 22, 2020
@MSeal MSeal added this to the no action milestone Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format:Slides pertains to exporting to the Slides format status:work-in-process Do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants