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

Img filename metadata #685

Merged
merged 4 commits into from Nov 13, 2017

Conversation

Projects
None yet
2 participants
@mpacer
Copy link
Member

mpacer commented Oct 3, 2017

closes #671

this should overwrite files if they are already present.

It also will add an extension to the file if it is not already present.

I'm open to changing either of those behaviours.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Oct 4, 2017

This looks fine to me. I might even go further and throw an error if two outputs in one notebook have the same filename.

@mpacer

This comment has been minimized.

Copy link
Member Author

mpacer commented Oct 12, 2017

How does that check feel? It looked like resources.ouptuts was acting as the global store of filenames, so this seemed like the right approach.

@takluyver
Copy link
Member

takluyver left a comment

The code looks good, thanks! I think the error message should explain a bit more, though.

@@ -102,6 +106,13 @@ def preprocess_cell(self, cell, resources, cell_index):
out.metadata.setdefault('filenames', {})
out.metadata['filenames'][mime_type] = filename

if filename in resources['outputs']:
raise ValueError(
"Your filename: {} appears more than once. "

This comment has been minimized.

@takluyver

takluyver Oct 13, 2017

Member

I think this message could be a bit clearer - imagine a user who has got a notebook from someone else and is trying to convert it. Even if someone wrote it themselves, they might accidentally duplicate a filename by copying a cell.

Let's mention that this is a filename associated with an output, which nbconvert is now saving to a separate file, and that the name has come from output metadata.

@takluyver takluyver added this to the 5.4 milestone Nov 13, 2017

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Nov 13, 2017

Love the detailed error message! :-)

@takluyver takluyver merged commit 9ec2fac into jupyter:master Nov 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment