remove linear search in get_base_map_name and get_base_hero_name + all other data lists. #5
Conversation
data/__init__.py
Outdated
| u"\u0422\u0440\u0435\u0439\u0441\u0435\u0440", u"\u730e\u7a7a", | ||
| u"\u9583\u5149 /// Tracer"], | ||
| "Tychus": [u"Tychus", u"Tychus", u"Tychus /// Tychus", u"Tychus /// Tychus", | ||
| u |
There was a problem hiding this comment.
so I'm not recreating to inverse mapping each time a new HeroUnit or Player class is created.
| u"\ub514\uc544\ube14\ub85c /// Diablo", u"Diablo /// Diablo", | ||
| u"\u0414\u0438\u0430\u0431\u043b\u043e /// Diablo", | ||
| u"\u8fea\u4e9a\u6ce2\u7f57 /// Diablo", u"\u8fea\u4e9e\u5e03\u7f85 /// Diablo"], | ||
| "Lunara": [u"Dryad", u"Lunara /// Lunara", u"Lunara /// Lunara", u"Lunara /// Lunara", |
There was a problem hiding this comment.
I don't get why you replaced the key from Dryad to Lunara.
There was a problem hiding this comment.
The keys are now the "canonical" name of the hero - so Lunara should be placed there. This is the same logic as previous code - if Dryad was the key and "Dryad" as inputted, we still would have outputted Lunara - see the previous code here: https://github.com/crorella/hots-parser/pull/5/files/ee572c1086ccffd6a211b41d2e3579db1608f621#diff-740b116474b5b8b4d5927ca113397b13L18
tests/data/test_data.py
Outdated
| assert HeroTranslator.get_base_hero_name("Gazleu /// Gazlowe") == "Gazlowe" | ||
| assert HeroTranslator.get_base_hero_name(u"Thrall /// Thrall") == "Thrall" | ||
| assert HeroTranslator.get_base_hero_name("Thrall /// Thrall") == "Thrall" | ||
|
|
There was a problem hiding this comment.
Can you please add tests for Gul'dan, Brightwing and Lunara? The reason is the internal representation for these heroes are Guldan, FaerieDragon and Dryad respectively and I would like to test those cases.
There was a problem hiding this comment.
Done - tests pass locally
|
Okay, I also tested locally and seems to be working fine, thanks @CamDavidsonPilon !! |
While not the least-performant part of the codebase, I wanted to clean up
get_base_hero_nameandget_base_map_name. They were doing inefficient loops over lists, and these can be much faster by using dict lookups. I've created classes for these lookups too, as they can encapsulate all the translation logic we require.Similarly, all the lists in
data/__init__.pycould be sets which offer betterinperformance.I've also add a test folder, I can run it locally with:
I also tested these changes locally in my database, things look good there too.
I'm most concerned about not catching unicodes correctly in production.