Add regional evolution metadata support #1299
Conversation
- Add region_restriction and base_form_required fields to PokemonEvolution model - Update PokemonEvolutionSerializer to include new fields in API responses - Add migration for new fields - Add implementation plan documentation - Add data import script for regional evolution data This addresses GitHub issue PokeAPI#639 by adding support for regional evolution requirements like Galarian Yamask → Runerigus and Galarian Slowpoke → Slowbro.
|
I think this is a no brainer, as currently the api has a huge oversight as far as how this is handled. This references a pokemon species for the form foreign key, it's my understanding that the species is the reference to the base forms data, though that object does contain a list of all forms. Due to this needing to reference a specific form, would it make more sense to instead link to the Pokémon id itself, and if species data is needed it can be queried from that Pokémon's species? This ensures the link is to the exact form required for the evolution, and does not require more implied logic to parse out based on the name passed with the species link. |
|
I'm away for some days and can't review these PRs at the moment. Thanks for submitting it |
|
Hi! Can you fix the CICD pipeline and continue your work with AI? It's promising. |
- Fix migration dependency to reference correct parent migration (0018_auto_20250123_1838) - Fix code formatting to match black standards: - Use double quotes instead of single quotes - Add trailing commas - Fix import formatting - Add proper spacing between functions
- Adjust spacing between functions in import_regional_evolutions.py for consistency. - Update unique_together definition in PokemonSummary model to use double quotes. - Ensure proper spacing in PokemonSummaryTextSerializer. - Add missing newline at the end of migration file 0020_add_regional_evolution_fields.py.
|
@Naramsim fixed the CI issues:
All CI checks are now passing. |
… evolution metadata - Updated v1beta and v1beta2 YAML files to include new foreign key constraints for region_restriction_id and base_form_required_id. - Enhanced EvolutionChainDetailSerializer to include region_restriction and base_form_required properties with appropriate schema definitions.
- Added steps to check pod, service, and job statuses before loading GraphQL. - Included commands to retrieve PokeAPI logs and test network connectivity. - Extended job wait timeout and added detailed debug information upon timeout.
|
@Naramsim Seeing that the loading of graphql is timing out. Added some debugging to see where it's getting hung up and also increased the timeout in case the CI environment is just slow. |
- Included checks for job existence and creation, along with status verification. - Added endpoint availability tests and enhanced logging for database migration and build processes. - Improved job wait process with additional status outputs and timestamps for better tracking.
- Implemented checks to determine if the load-graphql job has already failed, providing detailed debug information if so. - Added commands to retrieve job and pod statuses, along with logs from both init and main containers for better troubleshooting.
…phql image - Added checks for failed or error pods in the Kubernetes workflow, providing logs for troubleshooting. -image: debian:buster is end of life. Updated the load-graphql job to use the debian:bullseye image for improved compatibility.
- Eliminated unnecessary debug commands related to job, pod, and service status checks in the Kubernetes workflow. - Streamlined the process by retaining essential job wait functionality while enhancing clarity and efficiency.
|
@Naramsim I've been working the load-graphql issues and was able to resolve it. I see that the last dependabot commit (ee8ef9f) failed for the same reason. debian:buster was end of life and a switch to debian:bullseye in the load-graphql.yaml fixed the issue. Once this PR is approved the workflows should pass. Was able to test with github actions on my fork. |
|
@Naramsim What happens next? All checks passed:) |
|
Hi I want to check it out locally! And test it myself and understand the logic. Then I'll merge It in the staging branch and deploy it |
…ields appropriately, ensuring compatibility with existing data structures.
|
@Naramsim realistically, what is the reason to not just have these be separate species for the distinct forms? They have their own types, region, evolution. By not having the pokemon converge on the same species, when they realistically don't share the same data (color, generation, evolution, shape maybe) all of which are not shared with the other "forms" so it seems to be that these pokemon, which already exist as separate pokemon, should instead have their own species. The barrier to determine if a new species should be added should be if any of the foreign keys change for the form, in which case it should be a different species. This would organically fix this issue, and prep for whatever wild changes are coming down the line, especially as newer games more and more frequently add new forms/evolutions to pokemon. |
|
Hi @pauljoda no reason at all. We should get rid of all these species/forms and variety. We just don't have anyone willing to work on a possible schema and converting the existing data to it. |
|
I can take a crack at it, for reference I'm using the data for an iOS app, and actually used the Postgres dump to load it into a sqlite db to ship with the app, it quickly got way too network heavy to use the api calls. This would be a pretty big refactor, are you saying to flatten all forms, species, and varieties into the Pokémon table itself? I think there is some benefit to having a way for forms to share a common data with the root Pokémon, providing the form is mostly cosmetic. I do think that regional forms should be completely separate Pokémon. Perhaps, the structure of Pokémon/Species could still be used, but with the purpose now being Form/Pokemon, where the species table now takes the role of the base pokemon, and the pokemon table now takes the role of the differentiating factors per form. Any thoughts or suggestions before I work on that? I think that would be a far more future proof and simple method than adding a new table here, though this pr does solve the problem with the current API so might be worth pulling into for a quick fix and this change come in a future version. |
|
@pauljoda @Naramsim I understand there are changes people are looking to make that are a heavy lift. Can we wrap this one up while other work is being done? I would like to see how I can contribute to these other efforts but see one newer PR on top of this one. I also see a lot that are years old. Think we need to close these out and then come up with a strategy that works for the future changes. |
|
@joereg4 I'm sure it's fine, I think large changes like this should be for v3. I'll move this discussion to a thread so this can remain related for this change on v2 |
- Renamed `region_restriction` to `region` and `base_form_required` to `base_form` in the PokemonEvolution model for clarity. - Updated corresponding fields in the serializers and CSV to reflect the new naming conventions. - Adjusted foreign key constraints in the metadata YAML files to match the updated field names.
joereg4
force-pushed
the
regional-evolution-metadata
branch
from
2b66da2 to
bfe1e5b
Compare
|
@Naramsim Made the changes requested, please take a look:) |
|
Great! Thanks for sticking with us on this PR :) |
|
Could you also update the docs at pokeapi.co repo? |
|
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. |
4 participants
This addresses GitHub issue #639 by adding support for regional evolution requirements like Galarian Yamask → Runerigus and Galarian Slowpoke → Slowbro.