search

pysal / geopyter

GeoPyTeR: Geographical Python Teaching Resource
68 stars 17 forks source link

Change resource to nb #39

Open sjsrey opened 5 years ago

sjsrey commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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