KartikTalwar / Duolingo

Unofficial Duolingo API Written in Python
MIT License
825 stars 128 forks source link

small fix to get data for other users #82

Closed piggydoughnut closed 4 years ago

piggydoughnut commented 4 years ago

This is a small fix I made for myself in order to be able to get data for other users apart from the logged in one.

I saw there is a Pull Request which is working towards similar direction, but it doesn't seem active at the moment, so I am sharing this in case any one wants to use it.

igorskh commented 4 years ago

Nice and simple workaround.

Why do you need an originalUsername property in the class definition? For me, it looks more as a code duplication and would be cleaner to call set_username twice on an instance of the class.

Just a small hint :)

Function names should be lowercase, with words separated by underscores as necessary to improve readability.

Variable names follow the same convention as function names.

As for the Python style guide.

piggydoughnut commented 4 years ago

Noted.

I just thought the original username could be useful for someone, but I guess you are right since you can always get that info from the Duolingo responses.

Thx for the styleguide note, def something I need to study :)

piggydoughnut commented 4 years ago

It was actually not working :) coz I forgot to replace username in the user_url. Therfore I added a getter for the user_url.

Also there is a small issue now, in case the username that is being changed is not found on duolingo. I will fix that in a bit

SpangleLabs commented 4 years ago

Whoops, sorry about that! Misunderstood the new PyCharm UI for a sec there. Sorry about the delay, the world has been a bit hectic of late. I'm taking a look at this now

SpangleLabs commented 4 years ago

Okay, so looks good on this:

>>> duo = Duolingo("...","...")
>>> duo.get_languages()
['German']
>>> duo.set_username("Phillip549")
>>> duo.get_languages()
['Welsh', 'Spanish', 'German', 'Klingon']

but it seems to fail for get_daily_xp_progress() which is a bit odd.

For some nice easy tests, I added this to the bottom of tests.py

class DuolingoOtherUsernameTest(DuolingoTest):

    def setUp(self):
        self.lingo = duolingo.Duolingo(USERNAME, PASSWORD)
        self.lingo.set_username(USERNAME2)
        self.lang = self.lingo.user_data.learning_language

(get_audio_url() also seems to be failing again, but that's not your problem! Travis isn't really doing what I had hoped)

Looks like "https://www.duolingo.com/2017-06-30/users/{user_id}?fields=xpGoal,xpGains,streakData" returns an empty dict when given a user_id which isn't the authenticated user. We could make it check for an empty dict, and throw a meaningful error? Or maybe add that originalUser param back, and check before we make a request. Either way, might be worth updating the docs, hmm.

I can do those things after merge though, if you would prefer that, as this has been in PR a long time, sorry!

SpangleLabs commented 4 years ago

Anyway yeah, I can just merge this, and fix those things on master!

Many thanks!

SpangleLabs commented 4 years ago

Merged to master, added some tests, added some docs, little bit of extra error handling in get_daily_xp_progress(), and published to Pypi as version 0.5.0: https://pypi.org/project/duolingo-api/0.5.0/

Thank you tonnes for implementing this, sorry on the delay