Skip to content

Commit ae2616e

Browse files
programgamesclaude
andcommitted
@
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> @
1 parent 457211b commit ae2616e

2 files changed

Lines changed: 11 additions & 19 deletions

File tree

pokemon_v2/serializers.py

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from __future__ import annotations
44

55
import itertools
6-
import json
76
from typing import TYPE_CHECKING, Any, ClassVar, Protocol, cast
87

98
from django.db.models import Q
@@ -2942,29 +2941,22 @@ class Meta:
29422941
}
29432942

29442943
@extend_schema_field(PokemonSpritesSerializer)
2945-
def get_pokemon_sprites(self, obj: Pokemon) -> dict[str, str | None] | str:
2944+
def get_pokemon_sprites(self, obj: Pokemon) -> dict[str, Any]:
29462945
sprites_list = list(cast("PokemonWithRelations", obj).pokemonsprites.all())
29472946
if not sprites_list:
29482947
return {}
2949-
sprites: Any = sprites_list[0].sprites
2950-
if obj.pokemon_species is None or obj.pokemon_species.gender_rate != 8:
2951-
return sprites
2952-
if isinstance(sprites, str):
2953-
parsed = json.loads(sprites)
2954-
self._fill_female_sprites(parsed)
2955-
return json.dumps(parsed)
2956-
self._fill_female_sprites(sprites)
2948+
sprites = sprites_list[0].sprites
2949+
if obj.pokemon_species and obj.pokemon_species.gender_rate == 8:
2950+
self._fill_female_sprites(sprites)
29572951
return sprites
29582952

2959-
def _fill_female_sprites(self, node: object) -> None:
2960-
if not isinstance(node, dict):
2961-
return
2962-
sprites = cast("dict[str, Any]", node)
2953+
def _fill_female_sprites(self, sprites: dict[str, Any]) -> None:
29632954
for female_key, default_key in self._FEMALE_FALLBACKS.items():
29642955
if female_key in sprites and sprites[female_key] is None and sprites.get(default_key) is not None:
29652956
sprites[female_key] = sprites[default_key]
29662957
for value in sprites.values():
2967-
self._fill_female_sprites(value)
2958+
if isinstance(value, dict):
2959+
self._fill_female_sprites(value)
29682960

29692961
@extend_schema_field(PokemonCriesSerializer)
29702962
def get_pokemon_cries(self, obj: Pokemon) -> dict[str, str | None]:

pokemon_v2/tests.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1631,7 +1631,7 @@ def setup_pokemon_sprites_data(
16311631

16321632
pokemon_sprites = PokemonSprites.objects.create(
16331633
pokemon=pokemon,
1634-
sprites=json.dumps(sprites | {"other": {"showdown": showdown}}),
1634+
sprites=sprites | {"other": {"showdown": showdown}},
16351635
)
16361636
pokemon_sprites.save()
16371637

@@ -4523,9 +4523,9 @@ def test_pokemon_api(self):
45234523
"{}{}/pokemon-form/{}/".format(TEST_HOST, API_V2, pokemon_form.pk),
45244524
)
45254525

4526-
sprites_data = json.loads(pokemon_sprites.sprites)
4526+
sprites_data = pokemon_sprites.sprites
45274527
cries_data = json.loads(pokemon_cries.cries)
4528-
response_sprites_data = json.loads(response.data["sprites"])
4528+
response_sprites_data = response.data["sprites"]
45294529
json.loads(response.data["cries"])
45304530

45314531
# sprite params
@@ -4660,7 +4660,7 @@ def test_pokemon_api_female_only_sprites_fallback(self):
46604660

46614661
self.assertEqual(response.status_code, status.HTTP_200_OK)
46624662

4663-
response_sprites = json.loads(response.data["sprites"])
4663+
response_sprites = response.data["sprites"]
46644664

46654665
self.assertIsNotNone(response_sprites["front_default"])
46664666
self.assertEqual(response_sprites["front_female"], response_sprites["front_default"])

0 commit comments

Comments
 (0)