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 Cache.status property #193

Merged
merged 19 commits into from
Aug 28, 2022
Merged

Add Cache.status property #193

merged 19 commits into from
Aug 28, 2022

Conversation

BelKed
Copy link
Contributor

@BelKed BelKed commented Aug 23, 2022

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

BelKed commented Aug 23, 2022

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

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.

pycaching/cache.py Outdated Show resolved Hide resolved
@tomasbedrich
Copy link
Owner

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

BelKed commented Aug 24, 2022

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

cache_info["state"] = None # not on the page

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 43b3d3d

pycaching/cache.py Outdated Show resolved Hide resolved
@BelKed
Copy link
Contributor Author

BelKed commented Aug 24, 2022

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

@BelKed
Copy link
Contributor Author

BelKed commented Aug 24, 2022

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

test/test_cache.py Show resolved Hide resolved
pycaching/geocaching.py Show resolved Hide resolved
pycaching/cache.py Show resolved Hide resolved
pycaching/cache.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@FriedrichFroebel FriedrichFroebel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up.

@FriedrichFroebel FriedrichFroebel merged commit 33179c3 into tomasbedrich:master Aug 28, 2022
@BelKed BelKed deleted the cache-status branch August 28, 2022 13:36
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

Successfully merging this pull request may close these issues.

Cache.state does not correctly reflect the status of a geocache
3 participants