Closed alchemyst closed 7 years ago
Note that the reason coverage went down is because total lines of code in this well-covered module went down. The coverage in the files I worked on actually went up.
@johanzietsman @johanzietsman-em @christoffkok Have you had a chance to look at this? I'd like to merge it or reject it, but it's tedious to keep this up to date with the other things I've committed.
@alchemyst I took a look at this and I am all for it. This looks like a great change. There is less code and it is easier to understand and work with, and the tests pass. @johanzietsman-em is the original author of this. Just waiting for an 'ok' from him.
Incorporated a formula parser using parsimonious. I've kept the interface of
stoichiometry.py
exactly the same, but I've removed the redundant parsing code and replaced it with a single parser and some classes which represent the parsed bits. Instead of manual memoization, I've included thefunctools.lru_cache
on the parsing function, which should work the same way with less effort. This new parser is much easier to extend if we need to parse new forms in the future.I've tried to be relatively verbose in the comments, but I feel like parsing with the grammar is easier to understand than the previous hand-written parser. I'm putting this in as a pull request rather than just pushing because I know this is a big step and I want some eyes on this before merging.
Closes #191