[Issue 1124] Adds full imperial unit functionality to BMI Calculator

[bernardokoen]

Proposed Changes

Addresses #1124.

1. Displays appropriate units on BMI calculator for user based on unit preference.
2. Converts height and weight from imperial units to metric units when calculating BMI with imperial units selected.
3. Displays proper unit ranges in an error message when the user submits an inappropriate (out-of-range) height. Also gives a 406 error in this case.
4. Adds tests to ensure correctness of the above functionality.

Please check that the PR fulfills these requirements

• Tests for the changes have been added (for bug fixes / features)
• Added yourself to AUTHORS.rst

Other questions

• Do users need to run some commmands in their local instances due to this PR
  (e.g. database migration)?
  No

Changes made in collaboration with @gabeliss

[bernardokoen]

@rolandgeider Would love for you to take a look and let me know what you think when you get a chance. We were able to add these changes in to the existing framework without requiring much refactoring or restructuring.

[rolandgeider]

nice!

I'll try to take a closer look today or tomorrow, but this looks good. The only thing I'd ask you is to mark the error messages and other user facing strings as translatable

[bernardokoen]

Thanks! Please let me know if you have any other questions/concerns.

When you say mark the strings as translatable do you mean within the Pull Request or within the source code? Do you have an example we could reference? I dug around and was unable to find one.

[rolandgeider]

Sure, in nutrition/forms.py, the strings that are marked with _("foo"), then they can be later extracted and go to weblate so that the translators can translate it

[bernardokoen]

Thanks for sharing that. I've applied those changes to the PR. Let me know if that doesn't look right or anything else needs to be tweaked! Thanks for your help.

[rolandgeider]

don't need to translate the dictionary key :)

[rolandgeider]

Looks good, thanks! Just added a small comment, then we should be able to merge.

Right now you have to enter the height in inches which I'm guessing is probably not very usual? On the other hand adding a real parser to be able to read/write from/to 5'10" is probably a pain, so this can definitely stay

[bernardokoen]

We definitely agree–we considered using a foot and inches format but we thought it would add some unnecessary complexity and thought it would be best to keep in the same format as the cm. Took care of the remaining issue as well.

[bernardokoen]

@rolandgeider we're seeing some issues with the _ before the translatable strings not being recognized. Have you run into this before?

[rolandgeider]

you just need to import it 😄
from django.utils.translation import gettext as _

The "_" name is nothing special, just a convention to make it short so the strings can be read more easily

[bernardokoen]

Ah got it, thanks for clarifying. Just threw that in there.