4 ms·
> It's fine if you don't like it (although it may be you're just not quite used to it yet), but I'm not sure it's fair to call it "bad". It is totally fair to
by sweezyjeezy 8y ago
> It's fine if you don't like it (although it may be you're just not quite used to it yet), but I'm not sure it's fair to call it "bad".
It is totally fair to call a naming convention where all variables are 2 or 3 letter acronyms bad, at the very least un-pythonic, and certainly not suitable for an educational tool. If you think that it's good practice to make your code shorter at the expense your users needing an abbreviation guide, you're going to make adoption a much harder sell, especially if you're pitching at python devs. Unlike the last few decades, we now live in an age of autocomplete, there is absolutely no need for this.
Here is a few lines from the imdb ipython notebook (https://github.com/fastai/fastai/blob/master/courses/dl2/imdb.ipynb https://github.com/fastai/fastai/blob/master/courses/dl2/imd...), that gave me a headache just to look at -
trn_dl = LanguageModelLoader(np.concatenate(trn_lm), bs, bptt)
val_dl = LanguageModelLoader(np.concatenate(val_lm), bs, bptt)
md = LanguageModelData(PATH, 1, vs, trn_dl, val_dl, bs=bs, bptt=bptt)
The cognitive load put on someone unfamiliar with your code is not acceptable:
LanguageModelData takes two LanguageModelLoaders as arguments, and then later in the code it produces a model? Surely these two classes are named the wrong way round? You haven't explained what 'bs', or '1' are supposed to be. To figure out/ remember what the other variables are, you need to chase up and down the notebook, which would easily be avoided if you just gave them descriptive names. You only use these variables once or twice, so I don't even understand what is gained by making their names so terse.
- newen 8y agoRegarding the short variables names, I actually like it a lot. It's nice because you don't have to read as much. Once you are familiar enough with the abbreviations, it becomes much easier to read. Just from looking at this code, I can already tell you bs is batch size, bptt is back-projection through time. And then probably dl is data loader, lm is language model, md is model data. No idea what vs or 1 is but I can just look at the docs for LanguageModelData, which takes what? less than 30 seconds to read? I think it's worth it because you can just look at one line of code and know what's going on. Instead of parsing through a paragraph of code that does the same thing.