Skip to content

Conversation

StoicLoofah
Copy link
Collaborator

See the bug here

https://circleci.com/gh/ggtracker/sc2reader/157

ERROR: test_lotv_creepTracker (test_all.TestReplays)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/circleci/project/test_replays/test_all.py", line 434, in test_lotv_creepTracker
    assert replay.player[player_id].max_creep_spread >0
TypeError: '>' not supported between instances of 'tuple' and 'int'

The issue is that the value of max_creep_spread in this test is (900, 25.22680412371134), and python2 will allow you to compare the tuple and int and call it true, whereas python3 complains.

I looked at the code

if player.creep_spread_by_minute:
player.max_creep_spread = max(player.creep_spread_by_minute.items(),key=lambda x:x[1])
else:
## Else statement is for players with no creep spread(ie: not Zerg)
player.max_creep_spread =0

And there's actually an asymmetry of types here. For zerg, max_creep_spread is a tuple (like above), whereas for other races, it is the int 0. My belief is that these should match up, so either zerg should get

player.max_creep_spread  = max(player.creep_spread_by_minute.values())

or terran/protoss should get

player.max_creep_spread = (0, 0)

I'm not sure which if either is right, though, since I am concerned that ggtracker may depend on specific checks against either of these.

In any case, this hopefully will fix the unit test. If either @dsjoerg or @sklett-src could chime in, that would be helpful. Worst case is that I will just leave a comment int he plugin noting the asymmetry

@dsjoerg
Copy link
Member

dsjoerg commented Apr 29, 2018

GGTracker only cares about the second element of the tuple:
https://github.com/ggtracker/ggpyjobs/blob/master/sc2parse/sc2reader_to_esdb.py#L807-L809

@StoicLoofah
Copy link
Collaborator Author

@sklett-src do you have a preference on how I handle this?

@StoicLoofah StoicLoofah merged commit 1816755 into ggtracker:upstream May 14, 2018
@StoicLoofah StoicLoofah deleted the fix_max_creep_spread branch May 14, 2018 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants