pokebase icon indicating copy to clipboard operation
pokebase copied to clipboard

Add (caching) support for LocationAreaEncounters

Open jrubinator opened this issue 6 years ago • 4 comments

The pokeapi has an attribute that doesn't follow their typical NamedAPIResource pattern: LocationAreaEncounters. Per the docs, when looking up a pokemon's location_area_encounters, it returns a string (URL to list LocationAreaEncounter).

It would be really awesome if pokebase's API implemented lookup and caching for this nonstandard resource. I glanced over the code in hopes of submitting a PR, but as a relative python n00b, I'm having some trouble gauging how best to do this. I'd rather not re-implement caching for this API endpoint outside pokebase.

jrubinator avatar Mar 27 '18 01:03 jrubinator

FYI I requested background from the friendly folks on slack

jrubinator avatar Mar 28 '18 01:03 jrubinator

So here's the funny part ... the way location_area_encounters are shown in the PokeAPI docs, they should be automatically handled by creating NamedAPIResource objects (now APIResource objects). But they are implemented differently, so they are not handled by make_obj, hence why they need special handling now that I hadn't accounted for.

I've filed an issue on PokeAPI, but in the meantime I'll add special handling for them, to be included in the next release. (And then hopefully I can get a clear answer on what PokeAPI's planned way to handle them is, and clean up the wrapper code and the PokeAPI docs)

Good catch and thanks for the help! I'll close this issue once I write the fix and push the next release to PyPI.

GregHilmes avatar Jul 19 '18 03:07 GregHilmes

@GregHilmes - so I was looking at the current state of the changes, and noticed that there's a significant performance drop off from the changes:

Namely, once the cache is set, retrieving a pokemon takes an order of magnitude longer (jumping from taking ~.25s to ~3s).

 ~/programming/pokebase (pre-refactor)$ time python3 speed-test-old.py 

real	0m41.041s
user	0m0.796s
sys	0m0.065s
 ~/programming/pokebase (pre-refactor)$ time python3 speed-test-old.py 

real	0m0.241s
user	0m0.217s
sys	0m0.021s
 ~/programming/pokebase (pre-refactor)$ git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
 ~/programming/pokebase (master)$ time python3 speed-test.py 

real	0m31.404s
user	0m1.572s
sys	0m0.546s
 ~/programming/pokebase (master)$ time python3 speed-test.py 

real	0m3.095s
user	0m0.940s
sys	0m0.482s
 ~/programming/pokebase (master)$ cat speed-test.py 
import pokebase
from pokebase.cache import set_cache

set_cache('speed-test-cache')

pokebase.pokemon('jigglypuff')
 ~/programming/pokebase (master)$ cat speed-test-old.py 
import pokebase
from pokebase.api import set_cache

set_cache('speed-test-cache-old')

pokebase.pokemon('jigglypuff')

Let me know if you want a separate issue for that or anything.

jrubinator avatar Aug 19 '18 15:08 jrubinator

Good catch!

This should definitely be a separate issue - it has nothing to do with this issue. Please go ahead and file accordingly.

GregHilmes avatar Aug 19 '18 16:08 GregHilmes