veekun / pokedex

more than you ever wanted to know about Pokémon
MIT License
1.44k stars 637 forks source link

Adding past Pokemon type data? #288

Open phrasmotica opened 4 years ago

phrasmotica commented 4 years ago

Some background: I made a PR into PokeAPI that includes some new data. I was told that they consume data from this repo and that I should open an issue to have the new data integrated here before they merge my functional changes into PokeAPI.

The data describes a Pokemon's types in previous generations, and would go in a pokemon_types_past.csv file that follows the format of pokemon_types.csv but has an additional column generation_id, representing the last generation in which the Pokemon had the given type.

I'd really appreciate some feedback on this idea as this is data I'd like to use through PokeAPI in a project I'm working on 😄

My PR into PokeAPI is here: https://github.com/PokeAPI/pokeapi/pull/457

Thanks!

magical commented 4 years ago

I think the plan in the past was to add a generation column to pokemon_types, splitting it by generation (see issue #107). However, i suppose a changelog-like table could work. Types certainly don't change very often.

Any thoughts @encukou? I believe you wanted to get rid of all the changelog tables.

If we go this route, could you please model this after the existing changelog tables? In particular, the table should be named pokemon_type_changelog and the generation column should be named changed_in_generation_id.

How do you handle pokemon which gained a type rather than changing a type, like Magnemite?

phrasmotica commented 4 years ago

@magical I will work on making my changes follow the model you linked 🙂

In response to your second question, my changes would specify the exact typing that the Pokemon previously had. When querying its type in a given generation we first check if it has entries in pokemon_type_changelog that apply to that generation, and if it does then we disregard its entries in pokemon_type. So we solve the type addition case by treating the entries in the new table as direct replacements for those in the current table - a lack of an entry for a slot in the new table implies that the Pokemon had no type in that slot.

Taking Magnemite as an example, it would have a single entry in pokemon_type_changelog for its Electric type in slot 1 that applied until Gen I, implying that it hadn't yet earned its Steel type in slot 2.

phrasmotica commented 4 years ago

@magical I've made some prospective changes aligned with the existing ability changelog model, which can be viewed here: https://github.com/phrasmotica/pokedex/commits/feature/pokemon-types-changelog

encukou commented 4 years ago

(tl;dr: ignore this comment)

Any thoughts @encukou? I believe you wanted to get rid of all the changelog tables.

Yes: changelogs are a pain to query, unless you want to display a changelog. They're encoding the info in a way that's suitable for the web pokedex but not much else (including a potential different/improved web view). If I had the time, I'd go the Git way: store full state (and don't mind the cost, it won't be that big); generate diffs in the diff view.

But, don't mix refactoring with a data contribution. Veekun currently uses changelogs, and if that changes, types will just be one more small thing to adjust.

KidsDontPlay commented 4 years ago

What is the status here?