Closed robertodr closed 5 years ago
Few comments without having read everything:
user
.I like the simplification in the template. I was a bit unsure whether docstring is more intuitive than documentation. For Python developers it is.
I think the plan looks solid. Are there any portions of the code or any choices you are least sure about and would like input on?
OrderedDict
? (I think so, but I might be wrong)OrderedDict
: For "older" Python yes but 3.6 and later we don't need it. In 3.6 it is an implementation detail, in 3.7 it became a feature. EDIT: by "it" I meant insertion order is preserved.
About writing the reference dicts by hand - maybe we can write a script that will generate these and break these in controlled ways so that we can generate many dicts in one go for testing?
UPDATE I fixed this :smile:
~Lingering issues:- Default callables that mix types (like "'B3LYP' / 2"
) raise a SyntaxError
rather than a TypeError
. This is because the declared type is str
and the way I implemented closure execution treats strings differently (see the run_callable
function). This is the case for strings and complex numbers (as they are initially read in as strings). I don't know at the moment how to solve this (mild, I would say) inconsistency.~
There seems to be some trouble on Python 3.5 - let me know if I should look into that.
I switched to docstring
because I consistently fail to type "documentation" correctly... The Python 3.5 problem should be fixed now. Compatibility is hard!
This is a refactoring of the validation to fix, or offer a way to fix, #24, #26, #31, #33, and #34. This is still work-in-progress. I need to do step 7 and make sure that we have a good enough test suite. I am putting it up early because it might be a bit hard to digest "the festival of
dict
comprehensions". I have tried to document every function I have written in detail. Type hints might be wrong.The general strategy:
The fix for #24 is a byproduct of sorts to this restructuring: while traversing recursively if anything throws, we catch it and store how to get to the offending leaf and an appropriate error message. So for example, when the exception finally throws one will have something like this:
A drawback of this refactoring is that there's now a lot more functions and each of them does a recursive descent in the tree. There is also quite some code duplication: we might get rid of that using more higher-order functions.
TODO ~- [ ] Switch to
OrderedDict
throughout, to avoid annoyances with ordering of elements.~ Not needed