tomasbedrich / pycaching

A Python 3 interface for working with Geocaching.com website.
https://pycaching.readthedocs.io/
GNU Lesser General Public License v3.0
61 stars 46 forks source link

Add `Cache.status` property #193

Closed BelKed closed 2 years ago

BelKed commented 2 years ago

As discussed in #183, I'm opening a pull request to close this issue.

However, tests aren't written yet. There's a test script for now:

import pycaching

geocaching = pycaching.login("USERNAME", "PASSWORD")

caches = [
    "GC8CKQQ",  # Enabled
    "GC7Y77T",  # Disabled
    "GC1PAR2",  # Archived
    "GC10",     # Locked
]

for cache_wp in caches:
    cache = geocaching.get_cache(cache_wp)
    print(f"[https://coord.info/{cache.wp}] {cache.name} > {cache.status.name.capitalize()} • {cache.state}")

Fixes #183

BelKed commented 2 years ago

Now I found that it doesn't work properly with the Cache.load_quick() method. The received JSON states if the cache is available (it's not disabled nor locked) or if is archived. So, the locked status can't be determined. That means the complete load is needed. Or is it fine as it's now?


[https://coord.info/GC8CKQQ] Kohinoorka > Enabled • True
data['available'] = True
data['archived'] = False

[https://coord.info/GC7Y77T] Vojanovy sady > Disabled • False
data['available'] = False
data['archived'] = False

[https://coord.info/GC1PAR2] Kostel Panny Marie Ruzencove > Archived • False
data['available'] = False
data['archived'] = True

# Not actually archived, but locked
[https://coord.info/GC10] First Divine > Archived • False
data['available'] = False
data['archived'] = True
FriedrichFroebel commented 2 years ago

Thanks for your PR.

It seems like the CI tests are currently failing, although I did not have a deeper look into it.

Did you have a look at how to parse the data from the print page (_from_print_page)? As far as I can see, the status will not be parsed there? Or is it not available from this page at all?

As far as I can see it, if the short-comings of the quick loading functionality are documented correctly, this should not be a real issue, unless @tomasbedrich thinks different about it.

tomasbedrich commented 2 years ago

Thanks for seeking for my input. My general preference would be not to load attributes at all, if we are not able to load them correctly using given loading method. Then we can rely on lazy-loading.

BelKed commented 2 years ago

@FriedrichFroebel

It seems like the CI tests are currently failing, although I did not have a deeper look into it.

They are failing because I marked the Cache.state property as read-only (I removed the @setter method for it) as it's dynamically set based on the Cache.status. Some of the tests need to be updated.

Did you have a look at how to parse the data from the print page (_from_print_page)? As far as I can see, the status will not be parsed there? Or is it not available from this page at all?

The current status it's not there. It even wasn't parsed earlier with Cache.state: https://github.com/tomasbedrich/pycaching/blob/1f819261a7a89d2bf9799184e6db67cf76bc7c96/pycaching/cache.py#L148 I think it can be somehow guessed if we would also load logs (by adding &lc=10), but if there are more than 9 logs after the Archive log, it wouldn't be possible to find out. So, I wouldn't burden with that...

As far as I can see it, if the short-comings of the quick loading functionality are documented correctly, this should not be a real issue, unless @​tomasbedrich thinks different about it.

Already fixed based on his requirements in https://github.com/tomasbedrich/pycaching/pull/193/commits/43b3d3db9efae54cb47bd500709ad89999b5ed74

BelKed commented 2 years ago

Update: I fixed the tests and added some new ones, so CI should run now without any problems.

BelKed commented 2 years ago

Sorry, I forgot to sort the imports correctly...