Retrieve original values for ibu/fg/og/abv/color + Extra fields #20
Conversation
scheb
changed the title
|
@scheb Thanks for the PR. The changes look good to me. Triggered by your earlier PRs I took some time to clean up the code base, make the move to Python 3 and add more linting, formatting, publishing tasks for CI using GitHub Actions. This, plus your previous PRs, were released as version 2.0.0. Do you mind, rebasing/merging your PRs against the current |
|
Ok, I'll look into this. Is it itended that mypy is disabled in the Github actions? https://github.com/hotzenklotz/pybeerxml/actions/runs/276410625/workflow#L40-L43 Readme says I should run it and it displays quite some errors. Are you aware of this? |
Hahaha, you are vigilant as ever. Yes, I am aware. I had some trouble with these errors but I have fixed all typing issue by now. I also re-enable |
|
Alright, will update the PR later. Btw, since you seem to be interested in beers, here's the project I'm working on, that I'm using your library for: https://www.beer-analytics.com/ |
scheb
force-pushed
the
original-values
branch
from
ca46138
to
5a970df
Compare
|
Updated. I had to remove the "set_" from the setter methods because otherwise the recipe values wouldn't be set. |
| @@ -25,6 +27,18 @@ def __init__(self): | |||
| self._yield: Optional[float] = None | |||
| self.color: Optional[float] = None | |||
| self._add_after_boil: Optional[bool] = None # Should be Bool | |||
| self.version: Optional[int] = None | |||
There was a problem hiding this comment.
Any particular reason, why this is an int? What about versions like 1.2 or something crazy. (I haven't really seen anyone using the version property anyway)
There was a problem hiding this comment.
The spec says it's an int: http://www.beerxml.com/beerxml.htm
But I wouldn't mind to make it a Text for better compatibility. What do you think?
tests/test_parser.py
Outdated
| assert floor(recipe.ibu) == 25 | ||
| assert round(recipe.abv, 2) == 5.35 | ||
| # should have mashing base information | ||
| assert(type(recipe.mash) is Mash) |
There was a problem hiding this comment.
I am pretty sure PyLint will not like this. Probably:
| assert(type(recipe.mash) is Mash) | |
| assert isinstance(recipe.mash, Mash) |
There was a problem hiding this comment.
Change it. I also executed pylint against the tests folder and it showing some issues unrelated to the PR. It kept it as-is because I don't know how you want to deal with those things.
Updated the readme so that the commands include the tests folder.
tests/test_parser.py
Outdated
| assert round(recipe.abv, 2) == 5.35 | ||
| # should have mashing base information | ||
| assert(type(recipe.mash) is Mash) | ||
| assert (recipe.mash.name == "Single Step") |
There was a problem hiding this comment.
I guess all the parentheses here can be omitted. black should take care of that.
There was a problem hiding this comment.
Applied the black code style to the tests folder and updated the readme so that the commands include the tests folder.
|
Thanks again for the excellent pull request. Released as version We definitely need to hangout for a beer or homebrew at some point. |
|
Awesome, thank you! Yea we should have a beer at some point, I've seen your company is in Potsdam, that's no so far from Berlin ;) |
As discussed in #17 this is adding the ability to retrieve the original values from the recipe XML.
ibu/fg/og/abv/colorproperties:*_calculatedproperties:Other things to note:
gravity_to_platohelper function to reduce code duplication<MASH>is mapped to the object.nodes_to_objectwas used instead ofnode_to_object. I fixed that. I took the occasion to add some extra fields to theMashclass and added test cases.