Skip to content

Conversation

@larry-the-table-guy
Copy link
Contributor

Looks like posiongem was inherited from https://github.com/smogon/usage-stats/blob/09e4b930d73acf2def123f4b2f0bffb5156bb763/TA.py#L222. From that same source, focussash appears twice but removing that would require reformatting several lines.

@scheibo
Copy link
Contributor

scheibo commented Jan 27, 2025

Thanks! The snowwarning fix LGTM because that seems like a regression I introduced, but the posiongem fix should be guarded by the legacy flag (and documented in https://github.com/pkmn/stats/blob/main/stats/CHANGES.md) to remain bug-for-bug compatible with upstream.

EDIT: noticed smogon/usage-stats#15 - if that lands then this can be merged as is

@larry-the-table-guy
Copy link
Contributor Author

Now I found several typos inherited in the aliases table. I'm thinking of adding a test case here to assert that every ID in the table actually corresponds to a Pokemon in the Dex, and then fix those here and upstream.

@larry-the-table-guy larry-the-table-guy marked this pull request as draft January 29, 2025 05:09
@larry-the-table-guy larry-the-table-guy changed the title Stats: Fix typos in IDs in classifier Stats: Fix typos in IDs Jan 29, 2025
larry-the-table-guy added a commit to larry-the-table-guy/usage-stats that referenced this pull request Jan 30, 2025
@larry-the-table-guy
Copy link
Contributor Author

upstream aliases PR: smogon/usage-stats#16.

Also fixed an instance of arentrap here (not present in upstream). I'll have to check later whether it's ok to use the TRAPPING constants there or if it'll need a legacy flag.

@larry-the-table-guy
Copy link
Contributor Author

Original code also excludes pursuit, so gated it.

@larry-the-table-guy larry-the-table-guy marked this pull request as ready for review January 31, 2025 05:08
@scheibo scheibo merged commit 5072993 into pkmn:main Feb 1, 2025
1 check passed
@scheibo
Copy link
Contributor

scheibo commented Feb 1, 2025

Thanks for the great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants