New function to add Foods to the MFP DB

[Dnic94]

Hey there,

I hope this finds you well.

I have added a function to submit new food to the MFP database. I tried to respect your code style and use already implemented functions, logging and exception handling.

Feel free to merge or contact me with any questions or comments.

Greetings Dominic

[neilramaswamy]

I also just realized that we probably should add this to the documentation/README. I'd be happy to do so if you'd like, just let me know.

[Dnic94]

Hi Dominic, you have a fair number of whitespace changes in existing parts of this file. Did you mean to make those changes? For the doc string, it might be good to clarify that this is adding new food entries to the MFP DB (as in the PR title) to avoid confusion w/ logging a diary item. It might also be good for the docstring to mention the return value and its meaning. The trans_fat argument type definition has a typo, I think; as does servingspercontainer default value. On line 911 and 914 there is no placeholder for the error so the error will not be printed.

Hey Hannah, You are right. Fixed all the addressed issues, but whats the problem with 1.0 as default for servingspercontainer? The whitespace changes came from PyCharms reformat code on commit feature.

Thanks for the review.

[hannahburkhardt]

servingspercontainer: float = "1.0", You specify the datatype as float but the default value is a string :) In fact you also specify the other numbers to be of float type but then supply a string type default value. The type definition in python is a suggestion that is not enforced, but this wouldn't work in a strictly typed language... You might consider using None as default value, and checking for None when formatting, but since it doesn't really matter in python, I would consider that a style issue. When in doubt it's best to follow the existing style. From set_measurements, that would be specifying None as default value and formatting your post data without explicitly converting to a string, like so: "measurement[display_value]": value,

[Dnic94]

servingspercontainer: float = "1.0", You specify the datatype as float but the default value is a string :) In fact you also specify the other numbers to be of float type but then supply a string type default value. The type definition in python is a suggestion that is not enforced, but this wouldn't work in a strictly typed language... You might consider using None as default value, and checking for None when formatting, but since it doesn't really matter in python, I would consider that a style issue. When in doubt it's best to follow the existing style. From set_measurements, that would be specifying None as default value and formatting your post data without explicitly converting to a string, like so: "measurement[display_value]": value,

Just too obvious. 🙃 But for sure you are right. None as default value would be the correct way in my opinion but would also lead to a "bad" data-block for posting without previous checking and replacing None with "" again. I am not that familiar with all the format and styling options. I will follow your suggestions and have another look at the method which was used in set_measurements. Otherwise a sub-function like

def nonetoemptystr(value):
    if type(value) == None:
        return ""
    else:
        return value

should also work. Thanks again and have a nice day!

[Dnic94]

He,

I added some additional functions for

• Adjusting your nutrition goals
• Gather your meals
• Gather your recipes
• Gather meal details parsed into recipe schema from schema.org
• Gather recipe details parsed into recipe schema from schema.org

[hannahburkhardt]

Hi @Dnic94, since this is a pretty large change, I gave it a thorough-ish code style review. I hope you find my comments helpful and not just annoying...! Thanks for your contribution!

[Dnic94]

Hi @Dnic94, since this is a pretty large change, I gave it a thorough-ish code style review. I hope you find my comments helpful and not just annoying...! Thanks for your contribution!

Hey @hannahburkhardt, please feel free to leave your comments. Your feedback is always appreciated. :)

[Dnic94]

Thanks for taking the time to add a feature, @Dnic94! I think this would be really useful for a lot of people, and apologize for how many comments you're going to see below -- there are quite a few -- but they're mostly centered around a couple categories of suggestions:

1. Types are really helpful for folks who are using libraries like this since they make it clear what to expect when writing code that interacts with the library. Without them, folks have to read through the source and, in the worst case, run the code manually to see what to expect. That's really painful for people integrating with a library; so if I could trouble you to add return types to these functions below, I think folks using this would really appreciate it.
2. In general in Python, if you call a method, it either succeeds and gives you whatever it is you're asking for, or it fails and raises an exception. Here, in a handful of places, you're emitting logging messages to help communicate to the end user what went wrong, but unfortunately the library might not be being used by a person -- it might be wrapped up in some other application or script -- and there won't be anybody watching to read those logging messages. So, what I've suggested below in a variety of places is to either let an underlying exception go uncaught or to raise an exception of some kind when things fail. This is both helpful for folks integrating with this library, as well as folks just running code from the shell manually because that exception message will provide enough information to make an actionable bug report.
3. There are a handful of places where you have left commented-out code with the intention of helping future folks implement parts of these features. I totally get that you're trying to help a future person implement more functionality, but in most cases commented-out code just ends up being noise that gets in the way of folks understanding the actual uncommented logic around them, and in a lot of cases (as APIs drift and other changes occur) they become misleading over time. I've left suggestions here that you delete those lines to avoid that kind of problem.

Thanks again for taking the time to contribute a new feature, and I hope you forgive me for how many comments and suggestions you're going to see below.

Cheers!

Hey @coddingtonbear,

First I want to thank you for your great feedback and your time reviewing my code. I really appreciate it and I am thankful for any chance to learn new things or get a different view on things. I am not a professional developer so this is not my daily business.

I implemented most of your suggestions and implemented some missing things like docstrings and the so often mentioned return types. I also understand why we need to leave some exceptions uncaught which makes total sense from the view of a user of this library.

I respect your personal preferences to keep the code clean and therefore I removed all unnecessary debugging comments and code left overs.

I left some additional comments above in your commit suggestions. Everything should be fixed, explained or implemented now. Again thank you for your time and creating this library. I use it daily! Have a nice day!

[coddingtonbear]

Just to set expectations -- I've got a lot going on this week so I'm hoping to have a glance at this again this weekend sometime. Thanks for having taken the time to polish this up!