Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pokemon species from Paldea have wrong id #812

Closed
tillfox opened this issue Jan 10, 2023 · 30 comments
Closed

Pokemon species from Paldea have wrong id #812

tillfox opened this issue Jan 10, 2023 · 30 comments

Comments

@tillfox
Copy link
Contributor

tillfox commented Jan 10, 2023

The ids of pokemons from Paldea region are inconsistent with the actual numbering reported on serebii.net. This problem obviously exists even on other relations, like "pokemon_stats" and "pokemon". The ids should be changed evenly.

Paldea-Pokemons-Ids-Inconsistent

@NathanGuidry
Copy link

came here to say this. Everything is good for me up until #917.

@tillfox
Copy link
Contributor Author

tillfox commented Jan 12, 2023

came here to say this. Everything is good for me up until #917.

Yes, I can confirm. To solve this problem we can split the resolution in two steps:

  • Identify a mathematical relation (a set of couples (int, int)) that pairs the wrong id with the right one. This should be done manually, someone should check the wrong ids and report the correct ones, maybe in a csv file, in which every line is a couple of ids (wrong and right) separated by comma (oc). In short words someone should manually report all the wrong ids.
  • Create a script that assigns the right id to the tuples in the various tables (relations). This is the easy part (In term of waste of time), I can do this.

@NathanGuidry
Copy link

if you could be a little more specific with the formatting, I might be willing to help. would you want it like (wrongId1, rightId1), (wrongId2, rightId2), (wrongId3, rightId3), etc.?

@Naramsim
Copy link
Member

Hi, are you talking about the same issue @oneirocosm mentioned here?

#793 (comment)

Let me understand, the new pokemon have been released but their ID hasn't?

@tillfox
Copy link
Contributor Author

tillfox commented Jan 12, 2023

Hi, are you talking about the same issue @oneirocosm mentioned here?

#793 (comment)

Let me understand, the new pokemon have been released but their ID hasn't?

Yeah, the problem is the same mentioned there. Now we have the official ids, you can find them here : https://www.serebii.net/pokemon/nationalpokedex.shtml

@tillfox
Copy link
Contributor Author

tillfox commented Jan 13, 2023

if you could be a little more specific with the formatting, I might be willing to help. would you want it like (wrongId1, rightId1), (wrongId2, rightId2), (wrongId3, rightId3), etc.?

Thanks @NathanGuidry, the format should be:
wrong_id,correct_id
917,982
918,917
919,918
920,919
921,920
922,953
923,954
...

@NathanGuidry
Copy link

NathanGuidry commented Jan 13, 2023

Before I want to get started I want to point out an issue I created an issue (#817 ) that shows that #980 and #987 is not assigned to a pokemon at all. Just an FYI. I will get started on the csv. @tillfox

@NathanGuidry
Copy link

Here it is. Be wary of the null #980 and #987 that I mentioned earlier.
wrongIDs.csv

@Naramsim
Copy link
Member

If you know the correct ID feel free of submitting a PR fixing the order and the gaps

@tillfox
Copy link
Contributor Author

tillfox commented Jan 13, 2023

Here it is. Be wary of the null #980 and #987 that I mentioned earlier. wrongIDs.csv

Wow, good job @NathanGuidry! A fast work!

@tillfox
Copy link
Contributor Author

tillfox commented Jan 13, 2023

If you know the correct ID feel free of submitting a PR fixing the order and the gaps

Perfect, It will be easy to create a script to fix this, but there is a problem. This change will affect all these relations:

  • encounters (pokemon_id)
  • pokemon (id, species_id)*
  • pokemon_abilities (pokemon_id)
  • pokemon_form_generations (pokemon_form_id)
  • pokemon_form_names (pokemon_form_id)
  • pokemon_forms (id, pokemon_id)*
  • pokemon_species (id, evolves_from_species_id)*
  • pokemon_species_flavor_text (species_id)
  • pokemon_species_names (pokemon_species_id)*
  • pokemon_stats (pokemon_id)
  • pokemon_types (pokemon_id)
  • = Contains Paldea Pokemon data in master

Just the ones with star could give problems related to concurrency, but if someone else merges changes made using the wrong Ids from another branch, we'll have inconsistent data...

In addition to that, do I have permissions to directly push a branch on pokeapi, or should I just send you the updated files, so you can commit them? @Naramsim

@Naramsim
Copy link
Member

Hi, we'll need to change the ids of all the files at the same time.

There are currently two PRs that still use the old ids: #795 and #806. Done by @giginet

So I think the best is to wait for me to review those PRs and have them merged. Then we can run your script which should change the ids in many files.

I'd also like to know what @giginet thinks.

Currently, you @tillfox don't have any permission here on this repo. The best would be: wait for those PRs to be merged, fork PokeAPI/pokeapi, run the script, open a PR

@tillfox
Copy link
Contributor Author

tillfox commented Jan 13, 2023

Hi, we'll need to change the ids of all the files at the same time.

There are currently two PRs that still use the old ids: #795 and #806. Done by @giginet

So I think the best is to wait for me to review those PRs and have them merged. Then we can run your script which should change the ids in many files.

I'd also like to know what @giginet thinks.

Currently, you @tillfox don't have any permission here on this repo. The best would be: wait for those PRs to be merged, fork PokeAPI/pokeapi, run the script, open a PR

Ok @Naramsim, update me when I can go!
In the meanwhile I'm working on the script, It seems to work like a charm.

ScriptFixingPokemonSpecies

@tillfox
Copy link
Contributor Author

tillfox commented Jan 13, 2023

If you know the correct ID feel free of submitting a PR fixing the order and the gaps

Perfect, It will be easy to create a script to fix this, but there is a problem. This change will affect all these relations:

  • encounters (pokemon_id)

  • pokemon (id, species_id)*

  • pokemon_abilities (pokemon_id)

  • pokemon_form_generations (pokemon_form_id)

  • pokemon_form_names (pokemon_form_id)

  • pokemon_forms (id, pokemon_id)*

  • pokemon_species (id, evolves_from_species_id)*

  • pokemon_species_flavor_text (species_id)

  • pokemon_species_names (pokemon_species_id)*

  • pokemon_stats (pokemon_id)

  • pokemon_types (pokemon_id)

  • = Contains Paldea Pokemon data in master

Just the ones with star could give problems related to concurrency, but if someone else merges changes made using the wrong Ids from another branch, we'll have inconsistent data...

In addition to that, do I have permissions to directly push a branch on pokeapi, or should I just send you the updated files, so you can commit them? @Naramsim

@Naramsim sorry to bother you again, can you confirm me that these relations that I mentioned before are the only ones that contain ids of pokemons from Paldea? If you confirm this, I know that I have to focus only on these. Thanks in advance for the answer!

@giginet
Copy link
Contributor

giginet commented Jan 13, 2023

I read your discussions.
I think first it’s better to merge current PRs #795 and #806.

After mersing them their IDs should be remapped at all.

This process seems to be work well👍

@Naramsim
Copy link
Member

@Naramsim sorry to bother you again, can you confirm me that these relations that I mentioned before are the only ones that contain ids of pokemons from Paldea? If you confirm this, I know that I have to focus only on these. Thanks in advance for the answer!

Can't confirm, an analysis has to be done. And I don't have time 😥

@tillfox
Copy link
Contributor Author

tillfox commented Jan 13, 2023

@Naramsim sorry to bother you again, can you confirm me that these relations that I mentioned before are the only ones that contain ids of pokemons from Paldea? If you confirm this, I know that I have to focus only on these. Thanks in advance for the answer!

Can't confirm, an analysis has to be done. And I don't have time 😥

Got it! So, I made the script to fix the ids of Pokemons from Paldea in these relations:

  • encounters (pokemon_id)
  • pokemon (id, species_id)*
  • pokemon_abilities (pokemon_id)
  • pokemon_form_generations (pokemon_form_id)
  • pokemon_form_names (pokemon_form_id)
  • pokemon_forms (id, pokemon_id)*
  • pokemon_species (id, evolves_from_species_id)*
  • pokemon_species_flavor_text (species_id)
  • pokemon_species_names (pokemon_species_id)*
  • pokemon_stats (pokemon_id)
  • pokemon_types (pokemon_id)

If someone knows other relations that contains pokemons from Paldea with wrong ids, report them here, so I will extend the script!

Now, while the other pull requests are waiting to be accepted I'll refine and fix my script properly... Please notify me when I can apply the script, as soon as possible, before other data is added to the relations using wrong ids!

@tillfox
Copy link
Contributor Author

tillfox commented Jan 13, 2023

@NathanGuidry I spotted an error in the csv; Gholdengo id in pokeapi is 977, but it should be 1000. The line "977,1000" should be added to the file. if you can, please check if there are other things missing, so when I will apply the script there won't be errors left... Thanks in advance!

@NathanGuidry
Copy link

Apologies, I double checked all the data and that was the only error.
wrongIDs.csv

@tillfox
Copy link
Contributor Author

tillfox commented Jan 14, 2023

Apologies, I double checked all the data and that was the only error. wrongIDs.csv

Perfect, thanks!

@tillfox
Copy link
Contributor Author

tillfox commented Jan 14, 2023

Script done!

ScriptDone

@Naramsim
Copy link
Member

Hello everyone! @giginet's PRs are merged!

Can you update your master and re run your tests?

Many thanks!

@tillfox
Copy link
Contributor Author

tillfox commented Jan 16, 2023

Hello everyone! @giginet's PRs are merged!

Can you update your master and re run your tests?

Many thanks!

Done @Naramsim! I don't know if it was needed, but I did a Pull request too... Update me if something is wrong!

@tillfox
Copy link
Contributor Author

tillfox commented Jan 16, 2023

@Naramsim so is everything ok with data?

@Naramsim
Copy link
Member

@Naramsim so is everything ok with data?

I guess yes! Can you test a bit the API?

@simonorono you can submit the other sprites i think

@NathanGuidry
Copy link

NathanGuidry commented Jan 17, 2023

the https://pokeapi.co/api/v2/pokemon-species/ endpoints still return the incorrect pokemon including #980 and #987 in which still dont exist

@Naramsim
Copy link
Member

Hi! The API didn't get updated. Can you please try tomorrow?

It would be nice if you could also update the order

@PokeAPI PokeAPI deleted a comment from tillfox Jan 17, 2023
@Naramsim
Copy link
Member

Sorry, GitHub Mobile glitched and i saw 3 comments from @tillfox, all the same. So i delete one. But @tillfox only wrote one so i basically delete his comment.

@tillfox
Copy link
Contributor Author

tillfox commented Jan 17, 2023

Questa è censura @Naramsim! Apart from jokes, I'll fix it now I guess...

@tillfox
Copy link
Contributor Author

tillfox commented Jan 17, 2023

Ok, sad news, I can't fix this problem by script, because the order is something that has to be updated manually, since the order is not just related to pokemon ids, but to their evos and pre-evos too. So it needs a human intervention, or a more complicate script that I don't have time to write by now... I'll just close the issue and open a new one!

@tillfox tillfox closed this as completed Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants