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

Add (caching) support for LocationAreaEncounters #10

Open
jrubinator opened this issue Mar 27, 2018 · 4 comments
Open

Add (caching) support for LocationAreaEncounters #10

jrubinator opened this issue Mar 27, 2018 · 4 comments

Comments

@jrubinator
Copy link

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
Copy link
Author

jrubinator commented Mar 28, 2018

FYI I requested background from the friendly folks on slack

@GregHilmes
Copy link
Collaborator

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.

@jrubinator
Copy link
Author

@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.

@GregHilmes
Copy link
Collaborator

Good catch!

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

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

2 participants