-
Notifications
You must be signed in to change notification settings - Fork 264
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
Run black and isort over entire library #1203
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.
It's a bit overzealous in some places IMO, I'm not sure if we can pick and choose here.
return wrapper | ||
|
||
|
||
class ResultSet(): | ||
class ResultSet: |
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.
ResultSet(object)
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.
+1
+ (self.two_turns_after_good_defection_ratio) - 5) / 15)) | ||
+ (1 / (((len(self.history))+1) ** 2)) | ||
- (self.dict[opponent.history[-1]] / 4) | ||
( |
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.
this one is a bit much
turn == 41 and self.own_score < 113 or \ | ||
turn == 51 and self.own_score < 143 or \ | ||
turn == 101 and self.own_score < 293: | ||
if ( |
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.
IMO this one is better as
if turn == 11 and self.own_score < 23
or turn == 21 and self.own_score < 53
or turn == 31 and self.own_score < 83
or turn == 41 and self.own_score < 113
or turn == 51 and self.own_score < 143
or turn == 101 and self.own_score < 293:
@@ -1102,7 +1123,10 @@ def __init__(self): | |||
self.burned = False | |||
|
|||
self.defect_streak = 0 | |||
self.parity_streak = [0, 0] # Counters that get (almost) alternatively incremented. | |||
self.parity_streak = [ |
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.
# Counters that get (almost) alternatively incremented.
self.parity_streak = [0, 0]
@@ -1255,7 +1280,9 @@ def strategy(self, opponent: Player) -> Action: | |||
if turn % 15 == 0 and turn > 15: | |||
if self.detect_random(turn): | |||
self.mode = "Defect" | |||
return self.try_return(D, lower_flags=False) # Lower_flags not used here. | |||
return self.try_return( |
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.
# Lower_flags not used here.
return self.try_return(D, lower_flags=False)
@@ -1282,11 +1308,15 @@ def strategy(self, opponent: Player) -> Action: | |||
if self.detect_streak(opponent.history[-1]): | |||
return self.try_return(D, inc_parity=True) | |||
if self.detect_parity_streak(opponent.history[-1]): | |||
self.parity_streak[self.parity_bit] = 0 # Reset `parity_streak` when we hit the limit. | |||
self.parity_streak[ |
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.
same
self.parity_hits += 1 # Keep track of how many times we hit the limit. | ||
if self.parity_hits >= 8: # After 8 times, lower the limit. | ||
self.parity_limit = 3 | ||
return self.try_return(C, inc_parity=True) # Inc parity won't get used here. | ||
return self.try_return( |
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.
same
+ "{str_list[1]:^{width}}|" | ||
+ "{str_list[2]:^{width}}" | ||
) | ||
display_line = header_line.replace("|", ",") + ": {str_list[3]}," | ||
|
||
def make_commaed_str(action_tuple): |
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.
make_command_str
?
} | ||
|
||
def __init__(self) -> None: | ||
super().__init__() | ||
self.init_sequence = [ | ||
C, C, D, C, D, D, D, C, C, D, C, D, C, C, D, C, D, D, C, D | ||
C, |
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.
This was better as is IMO
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.
+1
for me, we either go for it lock, stock and barrel or we don't bother. If we start attempting to pick and choose, we'll give ourselves another maintenance task and we'll negate the whole point of using a automated tool to decide on formatting. |
Ok, I'm on board with that. |
Although I will poke around at some of black's options to see if there's anything that might make it a little less aggressive. |
Have you compared with autopep8 and other formatters? |
I agree that in some places it doesn't format how I would choose to format but I agree (definitely) with @meatballs, I think the advantage of using black is that it just means we don't need to exert energy/time on formatting. I'd suggest sticking with the defaults so that we don't even need to discuss picking and choosing them. Thanks for doing this @meatballs 👍 |
Can you add something to the documentation regarding that we use these autoformatters and briefly how users can follow suit? Do we want to add the formatters as a hook to be automatically applied (perhaps in another PR?). |
It's already added to the documentation in this PR. The automation, I was going to look into separately. |
I see that now but it doesn't say if we require that a PR be formatted in this way and/or that it is the canonical style that we except so please don't submit PRs that just change the formatting to a personal preference, etc. Otherwise LGTM |
I didn't want to be as prescriptive as saying that's the style we accept. We can always run the formatter on a file afterwards if anything needs cleaning up and, once it's automated, we don't really need to bother saying it at all. |
No description provided.