-
-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ship_view.py: Fix a few things, best I can figure out #144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good.
Please merge at the same time as the other fixes some stuff PR in engine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - left a comment on something that we could do a little better now that we're using Py3 - probably a little more efficient than the older methods too.
try: | ||
fl = float(ship_stats[key])/divider | ||
except KeyError: | ||
print('KeyError in get_dbl: ', key, ' not found in ship_stats') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI - since we're using Py3 we can use f-Strings here we makes formatting really nice and easy:
print(f'KeyError in get_dbl: {key} not found in ship_stats')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be f"blurb" and not f'blurb'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@royfalk it doesn't really matter which; typically you use one if you want to use the other:
f"blurb 'b' "
f'blurb "b" '
they both denote a string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Code Changes:
Issues:
Purpose: