Now with Formes!

[RichieBzzzt]

I noticed that new formes/mega evolutions were not included so I've added them. I've kept the names for the original 890 as they were and used forme names for the others. I've also added their stats. I've spot checked the stats relates to the correct form (I think Rotom has the most formes and it's stats are correct for all formes)

Some Pokémon (specifically Galarian SlowBro/Slowking) do not have artwork and/or stats so have had to add checks in retrieving stats and artwork.

I've replace the gender icons for the Nidorans to make it easier to type out.

Additionally, I changed the readme.md to point to pokemon\make_db on my PR from the other day, and I realised that I did not change that back, so it has been reverted back to scripts\make_db.

[RichieBzzzt]

Ah OK, catch_em_all returns the count including the formes as well. @vsoch how would you like me to fix; either alter the expected result of the test to 1030 or alter the function to only count for Pokemon not including formes?

[vsoch]

What is a formes? Is that a kind of Pokemon?

[vsoch]

And you are saying that the identifier includes text along with a number?

[RichieBzzzt]

Hiya,

"formes" is like the catch-all statement for "Pokémon with a major variations between individuals" (the weird spelling comes from Giratina.)

For example:

• Charizard has a "Mega Charizard X" and "Mega Chraizard Y" forme. Other Mega Evolutions are available...
• In addition to Marowak, there is an Alolan Marowak with diffrerent height, weight and typing
• Rotom has multiple formes
• Zygarde has three formes - 50%, 10% and Complete Forme

All of these variations have a different typing, height, weight and (crucially for the ascii generator) appearance, but with the same Pokedex number. And so for the sake of completion I have updated make_db to grab all of the formes images and stats.

Consequently this means that there's no longer a 1:1 ration with Pokemon entries as the forme variants share a Pokedex number with that of its base form, which is why we see 1030 Pokemon now instead of 890.

[vsoch]

Oh I see! I don't see any issue with that - I think the one bug that might be introduced is when we calculate the avatar, we are basing it on a number (instead of a number with a name for an index). I think if we can update the test to be the larger number, but then make sure that the calculation of the avatar only uses the "pure" (not Formes) set (meaning there is a number) then it should work!

[RichieBzzzt]

OK I'll update the test and verify the avatar calculation.

See comment below

[vsoch]

I think the cleanest thing to do would be to have a catch_base_pokemon() (or whatever you think is correct to call) that does the same as catch_em_all(), but only returns those with a numeric id (which should be numbers 1 through 890? Then when we generate the avatar here: https://github.com/vsoch/pokemon/blob/master/pokemon/skills.py#L54 we'd want to use that function. If we provide the length of the "true" number (1030) then if we try to index with a number we will get an error, as the numbers don't exist beyond 890. Does that make sense?

[RichieBzzzt]

I agree there is two distinct results here so makes sense to return two different values.

I'll make the change and get it pushed some time this week.

[RichieBzzzt]

So having looked over the code I can see that there should not be an overflow as each pokemon and forme are an entryi nthe database in their own right.

However because of this the [id does not correspond to the Pokémon National Dex Id EG get_pokemon(pid==25) will not return Pikachu but rather Rattata (owing to the extra formes). However big a problem this is depends on whether people want to get by pid or by name. We could alter this function to use lookup_pokemon , use the id field and return the first in the list, as this wil always be the original. Happy to do this if you think it is worth doing.

I also added some tests around the formes.

[vsoch]

My previous suggestion was to have the avatar generation be based on the previous set, and include the new types for other functions.

[vsoch]

The tests are fantastic! And I've just taken some time to think about this integration - and I think adding a new kind of (type? family?) warrants some discussion about the design of the software. Specifically:

• we don't want to break the current approach to calculate an avatar
• we want to allow for infinite expansion of new kinds/types (akin to forms) into the future and still have the software scale.

So here are some of my thoughts.

Organization

For forms (and any future addition corresponding to a set of images) I think we should have a modular organization, where each subfolder under "images" corresponds to a database. For example:

database/
     original/
     forms/
     forms.json
     original.json 

In the above, the current "images" folder maps to original, and the newly added forms maps to forms. The json files to load are each equivalently named, so if the user asks for original (which would be the default) we look to the original.json and folder of the same name. This is hugely flexible because it means that any additional set can be added, and just needs a unique id / descriptor for the folder and json name. If we are always getting these names/images from the same site, it would probably also be the case that the unique ids within folders are in fact unique between them (but I'm not sure). The reason I mention this is because a "catch_em_all" function could easily load all of them. Let's talk about this next.

Catch em All

The current catch em all just needs to load one database, and this is logical, but when we expand to add other types there are two approaches that we could take. Either we require the user to specify a specific folder/database (and default to original) OR default to None, and then serve a json that maps to all the subfolders. This would ensure that when we generate avatars, if we need to just get originals, we could do:

pokemons = catch_em_all(database="original")

or even extend to allow a list:

pokemons = catch_em_all(database=["original", "types"])

And have the default either be original, or None to indicate getting them all. That also brings up - we should have a function to look in the database folder and retrieve all available databases:

get_database_names()
["original", "forms"]

And then this would be exposed to the user on the command line if he/she wants to choose one

# the choices to the database argument are populated from the function
pokemon --catch --database original

Avatar Generation

The avatar generation should only be for the original set, so we would again do

pokemons = catch_em_all(database="original")

That is my early thinking on this - we want to be able to provide the original functionality, and not have to worry about updating one master databsae json / folder with a new set unless it's actually part of that set (your last PR). I have to run for a meeting, but I hope this is enough to spur your thinking and let me know what you think!