Units for target face diameter
jatkinson1000 opened this issue · 3 comments
Discussed in #17
- Add option for user to specify target face diameter unit
- Set default to be [cm]
- Store under the hood as [m] to preserve SI and not affect code elsewhere
This requires:
- Add optional argument to
Target
class - Add conversion code as part of
__init__
inTarget
class - Add
INCHES2M
andCM2M
toconstants
file - Add tests for conversion process
- Update and round data files accordingly
Initial change to the Target constructor is easy enough, have that drafted up.
But the question that arises is what do you want to do about Pass? At the moment it contstructs a Target from the raw parameters passed to it, so changing the default target diameter unit to cm has broken many, many things.
The simplest change is to just assume the units of the value passed to Pass are cm instead of m.
The other option is to replicate the api, give Pass a deafult diameter unit of cm as well and let it pass through to the Target constructor for conversion
Looked at it a bit more, was actually very easy to update all the tests to cm instead of m for diameters, and load_rounds just needed a single change to remove the division by 100. Can upload a pull request for this now
Yep, I just had a look and I think passing a "face size unit" to the pass constructor that goes through to the target constructor is the way to go.
Please do open a PR and I will do a review.
We can move this discussion over there (my development preference).