Replace custom cached_property with stdlib #1866
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1866 +/- ##
===========================================
- Coverage 86.00% 85.97% -0.03%
===========================================
Files 147 147
Lines 16049 16052 +3
===========================================
- Hits 13803 13801 -2
- Misses 2246 2251 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Oh dear. This works much better than I realised. It exposes more type errors! 😮💨 Going to have to come back to it. |
There was a problem hiding this comment.
There's something funky going on with types for SQLAlchemy (but the code itself is fine), so these 3 errors can be ignored for now:
datacube/drivers/postgis/_fields.py:83: error: "FromClause" has no attribute "dataset_ref" [attr-defined]
datacube/drivers/postgis/_fields.py:84: error: "FromClause" has no attribute "search_key" [attr-defined]
datacube/drivers/postgis/_fields.py:101: error: "FromClause" has no attribute "search_val" [attr-defined]
It's confusing to get these errors when making changes though, so it should be fixed, but I haven't had time to dig into that yet.
Reading the other error messages I'd say those look like real errors.
omad
force-pushed
the
replace-cached-prop
branch
2 times, most recently
from
6a9507a to
24e9669
Compare
| from collections import OrderedDict | ||
| from collections.abc import Iterable, Iterator, Mapping, Sequence | ||
| from datetime import datetime | ||
| from functools import cached_property |
There was a problem hiding this comment.
One of the cached properties in this file is assigned to in test_virtual.py:86, and another one in test_virtual.py:240. Is that supported by the functools cached_property, or could those two places be fixed easily so they populate the cached_property in some other way?
There was a problem hiding this comment.
Another question is if get_by_name_unsafe.cache_clear() & friends are related to cached_property and needs to be updated in some way when replacing the implementation?
There was a problem hiding this comment.
I'm not suggesting to keep the old cached_property around, but should the release notes perhaps mention that this was removed and tell the user to use the cached_property from functools instead?
Since Python 3.8 there's functools.cached_property, which works the same way, but also passes type information through.
The code works and is correct, typing sqlalchemy code is.. hairy
for more information, see https://pre-commit.ci
Since Python 3.8 there's functools.cached_property, which works the same way, but also passes type information through.
docs/about/whats_new.rstfor all changes📚 Documentation preview 📚: https://opendatacube--1866.org.readthedocs.build/en/1866/