As an nbdev user, it would be great to be able to tab complete flags and be able to access flag help without leaving the notebook I’m working on.
I’ve put a few ideas together here: https://github.com/pete88b/data-science/blob/master/fastai-things/nbdev/magic-flag-idea.ipynb but wanted to gauge interest before creating a pull request.
@jeremy / @sgugger / nbdev users; what do you think?
It sounds like a great idea @pete88b. There’s quite a bit of migration required, as you note, so we wouldn’t want to do it until after the course (mid-May). But if you want to experiment in a fork, that would be a good way to make progress in the meantime.
Hope the online course is going OK and you’re all keeping safe.
I’ve just got to shift a little lung infection and then I’ll get going with this
if anyone would like to take a early look, you could install a local copy of https://github.com/pete88b/nbdev/tree/magic-flags - existing projects can be migrated with nbdev_migrate2magic command.
any/all feedback would be welcome (o:
Sorry I didn’t have time to comment before. I replied on the PR but also pasting my answer here for people who don’t track GitHub.
This is a very consequential change that is not easy to implement now that nbdev is used as a fundation for fastpages. We can’t break backward compatibility and expect users to change their habits in a day.
At the same time, autocomplete with magic is a nice thing to explore, as is the help-display it can enable. So I don’t want to throw away the idea. I think the right direction would be to support both comments and magic commands, to save backward-compatibility and enable this cool feature. It would also make the migration easier.
Fair warning: I have almost no time to work on this right now and limited time to review. But if you’re up to it, we could merge this more progressively: just add a %nbdev_export first and make everything work with #export or %nbdev_export, check this all works well and progressively add the new flags.
I suppose I’ll copy/paste my comments on the PR here, too for visibility
I generally like the magic command approach and I see why it is appealing.
One thing that might give everyone some comfort is fastpages versions are pinned to specific versions of nbdev. So if this is eventually merged, it will not break fastpages unless people upgrade to a new version . In that case, I suppose we could make a conversion script that automatically executes upon an upgrade ( we can trigger this automatically if it is detected that someone is upgrading from version < 2.x to version > 3.x). That way we could mitigate the effect of any breaking changes.
TLDR; If we want to continue working on this, I think a conversion script to convert all notebooks to use magic commands is necessary to automatically mitigate breaking changes.
Let me know if anyone has any thoughts about this, in theory, a conversion script doesn’t seem like it would be that complicated.
My PR has a conversion script (
nbdev_migrate2magic) and a small change to existing nbdev commands that scans for comment flags and tells you if you need to migrate - I like the idea of auto-migration but thought we should let users make a conscious decision to migrate.
See https://github.com/pete88b/nbdev/blob/magic-flags/nbs/06_cli.ipynb for details
no need to apologize (o: I’m happy to create PRs one flag at a time and keep support for comment flags.
Let me know if I can lighten your load in any way: I’m looking for more challenges and I’m more than happy to contribute to fastai
I was suggesting that if we bump the version of nbdev we force the migration (via your script) to avoid breaking things? Or do we plan on maintaining both modalities forever (or for a long time)?
I think it makes sense to maintain both at the same time for a little bit: we still have bugs on fastpage that require some fixes in nbdev and I don’t want to go all the way down two branches with cherry-picks as a maintainer if I can avoid it.
Oh good point. I didn’t consider that. By the way what kind of bugs are these, or where can I find them? (I only see one open issue on the nbdev repo) If there are any bugs with fastpages I will gladly try to fix them
We addressed the existing ones, I’m talking about future ones
Please find the introduction of %nbdev_export https://github.com/fastai/nbdev/pull/153
this might look a little over-engineered for just one flag - but I think it’ll pay off - adding %nbdev_export_and_show should be a small change
This looks good at first glance, but I probably won’t have time to do a thorough review until next week. Thanks for being patient!