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

Change resource to nb #39

Open
sjsrey opened this issue Sep 19, 2018 · 2 comments
Open

Change resource to nb #39

sjsrey opened this issue Sep 19, 2018 · 2 comments

Comments

@sjsrey
Copy link
Member

sjsrey commented Sep 19, 2018

from @jreades

Currently we use 'resource' as the keyword inside the @include statement. That seems a rather wordy parameter name to me and I was wondering about 'nb' as an alternative that is also consistent with the terminology used by other utilities (nbconvert, etc.) that work with Jupyter notebooks.

@sjsrey sjsrey changed the title Change resource to nb Change resource to nb Sep 19, 2018
@sjsrey
Copy link
Member Author

sjsrey commented Sep 20, 2018

As far I can see, this would be a change in the specification, not the code, as I don't see where in the code we use "resource" as a keyword arg?

@jreades
Copy link
Collaborator

jreades commented Sep 25, 2018

Hmmm, strange.

It looks to me like the key method is:

def parse_include(self, include):

This is actually in the Cell class. I would need to dig into this a bit further to recall what’s going on, but if I had to make a guess I would say that I’ve made a stupid assumption about order:

nb = include[0].split("=")[1]

Which I’d say is poor form to say the least. That whole method looks like it needs to be rewritten to instantiate a dict and embed some requirements about key words that will throw errors otherwise.

Inline documentation of the code also looks rather poor now. :-P

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

No branches or pull requests

2 participants