change comment flags to magic flags

[pete88b]

if you're happy with these changes i'll create an equivalent PR against https://gitlab.com/thomas.capelle/nbdev_template/

[review-notebook-app]

Check out this pull request on 

Review Jupyter notebook visual diffs & provide feedback on notebooks.

---

Powered by ReviewNB

[pete88b]

@jph00 - please take a look when you get a minute

[jph00]

Thanks for the at-mention - sorry I didn't notice it before (and do feel free to mention me, since I don't always notice otherwise).

I'd rather leave comments instead of magic flags in the template. They're a bit easier to handle (i.e no need to import anything, no issues in colab, etc)

[pete88b]

@jph00 - if we don't update this template, would you like me to revert changes to the tutorial - which was updated on the assumption that "comment flags" would be deprecated - but before you say yes ...

Maybe this approach to working in colab might convince you that magic flags could be the preferred option?

I've put a little more detail in this forum discussion

[jph00]

@pete88b Personally I've found the flags awkward to use, and I haven't noticed any particular improvements for my particular workflow. I really tried to use them for a couple of months but in the end I just wasn't seeing benefits, and was seeing problems.

What's your thinking behind why they could be the preferred option in Colab?

[pete88b]

@jph00 until now, I thought that magics would be the preferred option everywhere - Sylvain and I had a plan to update fastai2 etc - so here are my thoughts on the benefits:

• Easier for people getting started with nbdev
  • I've got no evidence to back this up but ... i was hoping that being able to auto complete flags would help people discover other flags that they haven't used yet and make it easier/quicker to understand how they work
  • Showing usage errors in the nb gives immediate feedback - rather than seeing errors when running nbdev_build_lib etc
• Avoids accidentally using flags in comments
  • Made more likely by fastpage users that aren't using nbdev_build_lib
• I like how clearly magics show cell metadata

For me, the only downside is:

• I find magics slower to type then comments (even when running jupyter locally, i'm often too impatient to wait for autocomplete - so I end up typing more)
  • Maybe the nbdev_ prefix was a mistake?

I hear that you don't want to import nbdev just to get magic flags but ... this is fine for me - i always want notebook2script() at the end anyway.

Can you share other downsides and problems you've seen? i'm totally open to the idea that magics don't work that well - but i'd love to be able to find ways to improve everyone's nbdev/fastpages workflow and experience

[jph00]

I think there are pros and cons, and all the things you mention certainly make sense. For me, the issue really is that magics add an additional layer of complexity, and you can't even run a cell that contains them unless you've first imported a module. But they don't change the behavior of running a cell, so that's an unnecessary burden on the user IMO. It's also an extra concept the users have to learn - everyone already knows about comments, but people that's aren't too familiar with Jupyter won't know about magics, and the `%` thing is something they'll need to learn about. I had to revert the magic flags functionality because it was causing various build errors. In general, we need to be very careful of using `eval` - in fact we really should avoid it if at all possible! And we should also be very careful of running code directly in a module, other than defining functions and classes in the usual way. Having eval in code executed at imported is rather problematic, and I got so many confusing bugs I had to work around that I eventually decided I better just remove it! I think it should become just an `nbdev_flag` magic that takes an argument, where the argument is the name of the flag, if that's possible. I do agree that comments should not accidentally become flags! I suspect a good compromise here is to require the prefix `flag_` on all comment flags. Does that sound reasonable to you?

[pete88b]

Morning @jph00

I need a little time to think about changes to magics/comments but having just seen the bits you commented, I think the real problem you saw was that both building docs ExecuteShowDocPreprocessor and testing NoExportPreprocessor were running cells containing notebook2script - because we build docs in multiple processes, sometimes a process would import an empty module etc (because another process was part way through build lib).

The recently added _re_notebook2script check fixes this in both 03_export2html.ipynb and 04_test.ipynb.

Code complete for test flags is one of the best bits of magic flags for me - and I'm as sure as I can be that this code is solid.
I realize that this is a big ask but ... can I undo the changes to __init.py__ and flags.py to see if the _re_notebook2script check fixes your issues?

Here's the kind of error I think you were seeing:

converting: /home/peter/github/pete88b/nbdev/nbs/index.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/09_nbdev_callback_test.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/04_test.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/tutorial.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/08_flag_tests.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/07_clean.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/03_export2html.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/05a_conda.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/00_export.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/05_merge.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/01_sync.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/02_showdoc.ipynb
An error occurred while executing the following cell:
------------------
from nbdev.showdoc import show_doc
from nbdev.tutorial import *
------------------

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-386760f05a23> in <module>
----> 1 from nbdev.showdoc import show_doc
      2 from nbdev.tutorial import *

~/github/pete88b/nbdev/nbs/nbdev/__init__.py in <module>
      5 if IN_IPYTHON:
      6     from .flags import *
----> 7     from .showdoc import show_doc
      8     from .export import notebook2script

ImportError: cannot import name 'show_doc' from 'nbdev.showdoc' (/home/peter/github/pete88b/nbdev/nbs/nbdev/showdoc.py)
ImportError: cannot import name 'show_doc' from 'nbdev.showdoc' (/home/peter/github/pete88b/nbdev/nbs/nbdev/showdoc.py)

An error occurred while executing the following cell:
------------------
from nbdev.showdoc import show_doc
from nbdev.export2html import *
------------------

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-b9cc7cc9488b> in <module>
----> 1 from nbdev.showdoc import show_doc
      2 from nbdev.export2html import *

~/github/pete88b/nbdev/nbs/nbdev/__init__.py in <module>
      5 if IN_IPYTHON:
      6     from .flags import *
----> 7     from .showdoc import show_doc
      8     from .export import notebook2script

ImportError: cannot import name 'show_doc' from 'nbdev.showdoc' (/home/peter/github/pete88b/nbdev/nbs/nbdev/showdoc.py)
ImportError: cannot import name 'show_doc' from 'nbdev.showdoc' (/home/peter/github/pete88b/nbdev/nbs/nbdev/showdoc.py)

An error occurred while executing the following cell:
------------------
from nbdev.showdoc import show_doc
from nbdev.conda import *
------------------

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-ce4663247abf> in <module>
----> 1 from nbdev.showdoc import show_doc
      2 from nbdev.conda import *

~/github/pete88b/nbdev/nbs/nbdev/__init__.py in <module>
      6     from .flags import *
      7     from .showdoc import show_doc
----> 8     from .export import notebook2script

ImportError: cannot import name 'notebook2script' from 'nbdev.export' (/home/peter/github/pete88b/nbdev/nbs/nbdev/export.py)
ImportError: cannot import name 'notebook2script' from 'nbdev.export' (/home/peter/github/pete88b/nbdev/nbs/nbdev/export.py)

An error occurred while executing the following cell:
------------------
from nbdev.showdoc import show_doc
from nbdev.clean import *
------------------

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-fff520f946f7> in <module>
----> 1 from nbdev.showdoc import show_doc
      2 from nbdev.clean import *

~/github/pete88b/nbdev/nbs/nbdev/__init__.py in <module>
      6     from .flags import *
      7     from .showdoc import show_doc
----> 8     from .export import notebook2script

ImportError: cannot import name 'notebook2script' from 'nbdev.export' (/home/peter/github/pete88b/nbdev/nbs/nbdev/export.py)
ImportError: cannot import name 'notebook2script' from 'nbdev.export' (/home/peter/github/pete88b/nbdev/nbs/nbdev/export.py)

An error occurred while executing the following cell:
------------------
from nbdev.showdoc import show_doc
------------------

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-748d64ac7847> in <module>
----> 1 from nbdev.showdoc import show_doc

~/github/pete88b/nbdev/nbs/nbdev/__init__.py in <module>
      6     from .flags import *
      7     from .showdoc import show_doc
----> 8     from .export import notebook2script

ImportError: cannot import name 'notebook2script' from 'nbdev.export' (/home/peter/github/pete88b/nbdev/nbs/nbdev/export.py)
ImportError: cannot import name 'notebook2script' from 'nbdev.export' (/home/peter/github/pete88b/nbdev/nbs/nbdev/export.py)

An error occurred while executing the following cell:
------------------
from nbdev.showdoc import show_doc
from nbdev.merge import *
------------------

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-51c0dc00f42c> in <module>
----> 1 from nbdev.showdoc import show_doc
      2 from nbdev.merge import *

~/github/pete88b/nbdev/nbs/nbdev/__init__.py in <module>
      5 if IN_IPYTHON:
      6     from .flags import *
----> 7     from .showdoc import show_doc
      8     from .export import notebook2script

ImportError: cannot import name 'show_doc' from 'nbdev.showdoc' (/home/peter/github/pete88b/nbdev/nbs/nbdev/showdoc.py)
ImportError: cannot import name 'show_doc' from 'nbdev.showdoc' (/home/peter/github/pete88b/nbdev/nbs/nbdev/showdoc.py)

An error occurred while executing the following cell:
------------------
from nbdev.showdoc import show_doc
from nbdev.export import *
------------------

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-df42789a0f4b> in <module>
----> 1 from nbdev.showdoc import show_doc
      2 from nbdev.export import *

~/github/pete88b/nbdev/nbs/nbdev/__init__.py in <module>
      5 if IN_IPYTHON:
      6     from .flags import *
----> 7     from .showdoc import show_doc
      8     from .export import notebook2script

ImportError: cannot import name 'show_doc' from 'nbdev.showdoc' (/home/peter/github/pete88b/nbdev/nbs/nbdev/showdoc.py)
ImportError: cannot import name 'show_doc' from 'nbdev.showdoc' (/home/peter/github/pete88b/nbdev/nbs/nbdev/showdoc.py)


converting: /home/peter/github/pete88b/nbdev/nbs/99_search.ipynb
converting: /home/peter/github/pete88b/nbdev/nbs/06_cli.ipynb

ERROR:root:Internal Python error in the inspect module.
Below is the traceback from this internal error.

Conversion failed on the following:
03_export2html.ipynb
07_clean.ipynb
00_export.ipynb
tutorial.ipynb
08_flag_tests.ipynb
05_merge.ipynb
05a_conda.ipynb
Traceback (most recent call last):
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/site-packages/IPython/core/interactiveshell.py", line 3331, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-92-85db608c675b>", line 6, in <module>
    assert all([f.exists() for f in dest_files])
AssertionError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/site-packages/IPython/core/interactiveshell.py", line 2044, in showtraceback
    stb = value._render_traceback_()
AttributeError: 'AssertionError' object has no attribute '_render_traceback_'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/site-packages/IPython/core/ultratb.py", line 1148, in get_records
    return _fixed_getinnerframes(etb, number_of_lines_of_context, tb_offset)
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/site-packages/IPython/core/ultratb.py", line 316, in wrapped
    return f(*args, **kwargs)
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/site-packages/IPython/core/ultratb.py", line 350, in _fixed_getinnerframes
    records = fix_frame_records_filenames(inspect.getinnerframes(etb, context))
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/inspect.py", line 1502, in getinnerframes
    frameinfo = (tb.tb_frame,) + getframeinfo(tb, context)
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/inspect.py", line 1460, in getframeinfo
    filename = getsourcefile(frame) or getfile(frame)
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/inspect.py", line 696, in getsourcefile
    if getattr(getmodule(object, filename), '__loader__', None) is not None:
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/inspect.py", line 739, in getmodule
    f = getabsfile(module)
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/inspect.py", line 708, in getabsfile
    _filename = getsourcefile(object) or getfile(object)
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/inspect.py", line 687, in getsourcefile
    if any(filename.endswith(s) for s in all_bytecode_suffixes):
  File "/home/peter/anaconda3/envs/nbdev-fork/lib/python3.7/inspect.py", line 687, in <genexpr>
    if any(filename.endswith(s) for s in all_bytecode_suffixes):
AttributeError: 'PosixPath' object has no attribute 'endswith'

---------------------------------------------------------------------------

[jph00]

The recently added `_re_notebook2script` check fixes this in both 03_export2html.ipynb and 04_test.ipynb. Code complete for test flags is one of the best bits of magic flags for me - and I'm as sure as I can be that this code is solid. I realize that this is a big ask but ... can I undo the changes to `__init.py__` and `flags.py` to see if the `_re_notebook2script` check fixes your issues?

Can you re-write it to patch the module directly, rather than using `eval`?

[pete88b]

@jph00 done AnswerDotAI/nbdev#232 please let me know if you see any problems or have any test scenarios you need me to run through.
I run all notebooks, nbdev_build_lib, nbdev_test_nbs and nbdev_build_docs --force_all=True multiple times on my machine - and i'm about to do the same in colab