Sniper task: Keeping 'missing' Pokemon + sorting order correction #5613
Conversation
- A 'missing' only Pokemon (not on the catch_list, no VIP and with a bad IV) will now be keep on the list as a potential target if 'missing' is specified on the order option - The sorting of the targer list is modified to be cumulative. For example, if the `order` option is [`missing`, `vip`, `priority`], then the list will be ordered by 'missing' first, and then VIP's, and finally by Priority.
Snipe task: Keeping 'missing' Pokemon + sorting order correction
Import itemgetter
Adding import
|
@GhosterBot, thanks for your PR! By analyzing the annotation information on this pull request, we identified @YvesHenri to be a potential reviewer |
GhosterBot
changed the title
| @@ -277,8 +278,11 @@ def is_snipeable(self, pokemon): | |||
| if pokemon.get('vip', False): | |||
| self._trace('{} is not catchable and bad IV (if any), however its a VIP!'.format(pokemon.get('pokemon_name'))) | |||
| else: | |||
| self._trace('{} is not catachable, nor a VIP and bad IV (if any). Skipping...'.format(pokemon.get('pokemon_name'))) | |||
| return False | |||
| if pokemon['missing']: | |||
There was a problem hiding this comment.
Please change it from if pokemon['missing']: to if pokemon.get('missing', False):.
There was a problem hiding this comment.
Ho I see, sorry. Thanks to have handle that
| @@ -362,8 +366,7 @@ def work(self): | |||
|
|
|||
| if targets: | |||
| # Order the targets (descending) | |||
| for attr in self.order: | |||
| targets.sort(key=lambda pokemon: pokemon[attr], reverse=True) | |||
| targets = sorted(targets, key=itemgetter(*self.order), reverse=True) | |||
There was a problem hiding this comment.
This is and targets.sort(key=lambda pokemon: pokemon[attr], reverse=True) are the same thing (and requires less imports).
There was a problem hiding this comment.
@YvesHenri
Not exactly, because this method is called as many times as there is attr in self.order: , and that's the problem.
So, at every call on the loop, the targets list will be sorted by loop's current pokemon[attr] only.
So if there is 4 orders specified (missing, priority, iv, vip), the targets.sort will be called 4 times, and everytimes the sorting will be on one attribute only. First missing only, then priority only, then iv only, and finally ordering by vip only. So finally the list will only be sorted by vip (in this example), regardless the others options.
I see it with my detailed logs .
So calling
targets = sorted(targets, key=itemgetter(*self.order), reverse=True
will do in one pass a cumulative ordering, thanks to the *self.order parameter where all the desired order will be handle at once (look at my screenshot for another example)
Please look at https://docs.python.org/2/library/operator.html :
-> Changed in version 2.5: Added support for multiple item extraction.
That's exactly what we are looking for
There was a problem hiding this comment.
I know. Thats what I meant. Although, the final/merged list will have around 3050 items, and if you have a max of 5 ordering types, that should be an iteration of 150250 times, which is worthless. I'll accept it anyways.
Changing attribut call
| @@ -362,8 +366,7 @@ def work(self): | |||
|
|
|||
| if targets: | |||
| # Order the targets (descending) | |||
| for attr in self.order: | |||
| targets.sort(key=lambda pokemon: pokemon[attr], reverse=True) | |||
| targets = sorted(targets, key=itemgetter(*self.order), reverse=True) | |||
There was a problem hiding this comment.
I know. Thats what I meant. Although, the final/merged list will have around 3050 items, and if you have a max of 5 ordering types, that should be an iteration of 150250 times, which is worthless. I'll accept it anyways.
|
👍 |
For example, if the order option is [missing, vip, priority], then the list will be ordered by 'missing' first, and then VIP's, and finally by Priority. Actually, the list is only ordered by the last order option specified on the config (only by priority on this example)
Screenshot example of a cumulative [missing, priority, iv, vip] order list