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

Add ExtractAttachmentsPreprocessor #1978

Merged
merged 5 commits into from
May 6, 2023
Merged

Conversation

tuncbkose
Copy link
Contributor

@tuncbkose tuncbkose commented Apr 19, 2023

Adds a new preprocessor to extract notebook attachments into individual files, so that conversion outputs (like .md/.tex files) work properly.

Fixes #1977.

Todos

  • Finish implementation
  • Finish preprocessor tests
  • Add tests related to FilesWriter tweaks

@tuncbkose tuncbkose force-pushed the attachments branch 2 times, most recently from 183c3bb to 32be570 Compare April 19, 2023 13:57
@tuncbkose
Copy link
Contributor Author

The previous iteration was fine with the tests, but it looks like the way I associated attachment directories with specific notebooks broke something. Will look into it soon.

@tuncbkose
Copy link
Contributor Author

I am not sure what is the best way to enable this preprocessor. I initially thought of adding it to TemplateExporter.default_config but ran into #1980.

For now, I enabled it for markdown and latex exporters.

@tuncbkose
Copy link
Contributor Author

It might be worth noting that I am aware ExtractOutput is pretty similar, but I am under the impression that cell outputs and attachments are different enough mechanisms to warrant different preprocessors. Still, they should probably work relatively similarly, for example if a directory is created to save related files.

I'll take a look to see how it works and make appropriate changes later.

@krassowski krassowski added the bug label Apr 22, 2023
@tuncbkose tuncbkose force-pushed the attachments branch 5 times, most recently from 59ee3c9 to b14e8d3 Compare April 25, 2023 08:48
@tuncbkose tuncbkose marked this pull request as ready for review April 25, 2023 08:57
@tuncbkose
Copy link
Contributor Author

Updated with latest commits to main

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
@blink1073 blink1073 added enhancement and removed bug labels May 6, 2023
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attempting to convert a notebook with pasted image to PDF fails with an error
3 participants