Closed jayqi closed 4 years ago
@pjbull @r-b-g-b this PR is ready.
Apologies that it is quite hefty, but the new functionality I wanted to add really required some significant refactoring. Also a lot of it is tests! This PR is at 99% test coverage.
Merging #30 into master will increase coverage by
3.58%
. The diff coverage is98.54%
.
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 94.73% 98.31% +3.58%
==========================================
Files 2 8 +6
Lines 152 357 +205
==========================================
+ Hits 144 351 +207
+ Misses 8 6 -2
Impacted Files | Coverage Δ | |
---|---|---|
nbautoexport/export.py | 95.52% <95.52%> (ø) |
|
nbautoexport/jupyter_config.py | 96.36% <96.36%> (ø) |
|
nbautoexport/__init__.py | 100.00% <100.00%> (ø) |
|
nbautoexport/__main__.py | 100.00% <100.00%> (ø) |
|
nbautoexport/clean.py | 100.00% <100.00%> (ø) |
|
nbautoexport/nbautoexport.py | 98.91% <100.00%> (+4.35%) |
:arrow_up: |
nbautoexport/sentinel.py | 100.00% <100.00%> (ø) |
|
nbautoexport/utils.py | 100.00% <100.00%> (ø) |
|
... and 4 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 31a9d6f...a44bae0. Read the comment docs.
@pjbull @ejm714 @r-b-g-b
Okay, I think this should be ready to go.
export
and convert
commands. This is just named export
. pdf
from the unit tests because unfortunately it requires xelatex
, and I couldn't figure out a way to easily install it in the CI. ~Will open a new issue about figuring this out.~ Opened #35. Ultimately, testing pdf
had helped me catch a bug last week, so I would like to have it tested.
Added new commands to the CLI. The summary of the commands are as follows:
clean
: Removes files that don't match whatnbautoexport
would create based on current notebooks and current config. For example, the use case is removing stuff likeUntitled.py
to clean up after stuff from renamed or deleted notebooks.convert
: This runs conversion with the configuration provided by CLI arguments (no .nbautoexport file needed).~export
: This runs conversion on a folder with a .nbautoexport configuration file or with override CLI options.install
: The old main command of creating a .nbautoexport configuration file.Additional changes:
clean
configuration option to automatically run the cleaning as part ofpost_save
. Defaults toFalse
.rst
as an export format__main__.py
to support for calling withpython -m
. This requires settingtyper>=0.3.0
.Additional refactoring:
export.export_notebook
to better support code reuse forconvert
andexport
commands.os.path
to usepathlib
How stuff works:
clean
, ~convert
~, andexport
know what files are notebooks:utils.find_notebook
will attempt to load every file in given directory withnbformat
.clean
knows what files to expect:clean.notebook_exports_generator
holds logic for predicting exported files.clean.get_extension
grabsnbconvert
exporters to figure out appropriate file extensionFixed some bugs:
asciidoc
,latex
,markdown
, andrst
formats create separate directories for image files that weren't getting moved into the subfolders.pdf
format resulted in a unicode decode error because the postprocessor was trying to read the pdf file back in as unicode text in order to remove the cell numbersnbconvert
grabs CLI arguments fromsys.argv
, which leads to really weird errors ifnbconvert
gets initiated through something likepytest
ornbautoexport
. Previously this was worked around in the tests by monkeypatchingsys.argv
. Instead, we now use a context manager to temporarily clearsys.argv
whenevernbconvert
is initialized, which prevents errors no matter if it wasnbautoexport
,pytest
, orjupyter
that starts the outer process.Looking for feedback:
convert
andexport
could be confusing.~install
is what the original function for setting up was named this whole time. Are there better names?configure
?watch
? I personally likewatch
.~ Opened issue #34organize_by
ofnotebooks
andextensions
. Wondering if it would make sense to supportNone
, i.e., keep converted output in parent directory and not in subfolders.~ Opened #32TODO:
Known issues, thinking will save these for the future:
script
.nbconvert
actually determines the language from loading the.ipynb
file and then dispatches to the specific language exporter. We may want to do the same thing.~ ended up implementingnbconvert
library only directly supportspython
. We'll have to figure out how it knows about others. Currently this will return.txt
.~ Ran this against an R notebook I had, and it looks up the.r
from the notebook metadata underfile_extension
.asciidoc
,markdown
, andrst
create for images, soclean
will remove them. This may require figuring out naming conventions, and/or refactoring theexpected_exports
method to return globs instead of Paths.~ ended up implementingclean
,export
,convert
) currently just glob for*.ipynb
files. This may miss stuff, and we should potentially instead try checking any file withnbformat
instead.~ ended up implementingNote: I tried to get the root of the CLI to point to
install
but it doesn't work, because other subcommands will conflict with arguments.Resolves #20, #21