feat: fallback front_female sprite to front_default for female-only p… - #1573
Conversation
379ce9e to
416c268
Compare
| serializer_class = PokemonDetailSerializer | ||
| list_serializer_class = PokemonSummarySerializer | ||
|
|
||
| def get_queryset(self): |
There was a problem hiding this comment.
Very interesting implementation. Can I ask what's the purpose of this function? It's for having ready at hand some data later on in the serializer.py part?
There was a problem hiding this comment.
Hi, @programgames caould you address the above question?
| "back_shiny_female": "back_shiny", | ||
| } | ||
|
|
||
| def _fill_female_sprites(self, node): |
There was a problem hiding this comment.
I'm strongly of the opinion that the data should just be shared with PokeAPI and consumers should handle this themselves.
There was a problem hiding this comment.
Among the years we always pulled data from veekun which in turn got it from decompilations. The data was sometimes pretty cryptic to use. I can mention the use of rare Unicode characters, having braces in strings that acted as templates, having hidden characters and so on. In the beginning we were reluctant to change these pieces of data turning to the users to implement custom strategies.
Eventually we cleaned the garbage that we were serving and everyone was happier. Maybe valuing user-friendliness over strict data rules was the key to it.
That said I'm in favour of these kind of changes. If a pokemon has a 100% female rate then the female_sprite can be inferred to be the same as the default_sprite. In this case default_sprite and female_sprite coincide. By having the female_sprite filled in (by this custom logic or any other method) is ok with me.
|
Free feel to ping me if changes are needed. |
| sprites = sprites_object.sprites | ||
| if obj.pokemon_species.gender_rate == 8: |
There was a problem hiding this comment.
@programgames would you be able to update your code and fix the conflict in this file? Recently @FallenDeity heavily rewrote it and I wasn't able to fix the conflict by myself.
…e-only pokemon For pokemon whose species has gender_rate=8 (exclusively female), fill the null female sprite keys with their default counterparts across all sprite sections. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5888657 to
457211b
Compare
refactor: simplify female sprite fallback per review The sprites model field is a JSONField, so it is always decoded to a python dict on read: drop the string/json.loads/json.dumps branch which would have returned an escaped JSON string in the response. Simplify get_pokemon_sprites to a positive guard and narrow its return type to dict[str, Any]. In _fill_female_sprites, type the parameter as dict[str, Any] and move the isinstance check into the recursion loop instead of guarding at the function entry, which also removes a cast. Align the test fixture with data/v2/build.py: it double-encoded the sprites dict with json.dumps before storing it in the JSONField. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> @
|
everything looks good to me but please add the ai usage disclaimer/disclosure in your pr desc we have a template here: https://github.com/PokeAPI/pokeapi/blob/master/.github/pull_request_template.md |
|
A PokeAPI/api-data refresh has started. In ~45 minutes the staging branch of PokeAPI/api-data will be pushed with the new generated data. |
|
The updater script has finished its job and has now opened a Pull Request towards PokeAPI/api-data with the updated data. |
its live now thx @programgames |
5 participants
Changes description
A simple solution for the #271 .
If the request if a simple GET for the pokemon, we select the specie and check if the gender rate is only female.
In this case we copy the valuer from default to female sprites with this mapping :
PS : I would like to know if Pokemon.objects.all().select_related("pokemon_species") could be a performance problem on the server, Should I test performance before and after this change to compare ?
AI coding assistance disclosure
I used AI to improve my indentation after my manual changes, make a test and understand/fix from PR reviews.
I also undestand what the AI did to my code.
Contributor check list