telluric icon indicating copy to clipboard operation
telluric copied to clipboard

GeoVector.__eq__crash on inconsistent types

Open penguinolog opened this issue 6 years ago • 6 comments

Expected behavior and actual behavior

from telluric import GeoVector, constants
from shapely import geometry
from rasterio.crs import CRS

poly = geometry.Polygon.from_bounds(4334065.7375318575650454, 5627100.9060072815045714, 4337359.6036174902692437,5630885.4445580849424005)

roi = GeoVector(poly, crs=constants.WEB_MERCATOR_CRS)
roi == 1

will crush with AttributeError instead of return False

OS independent, version = trunk.

penguinolog avatar Aug 14 '18 16:08 penguinolog

@penguinolog is it common practice to return False as result of comparison of incompatible types? For me personally it looks like problem masking and making debugging more complicated.

drnextgis avatar Aug 14 '18 16:08 drnextgis

It's common practice to make simple compare for equality and separately make type check. Sometimes not exactly the same types are equal and sometimes not.

penguinolog avatar Aug 14 '18 16:08 penguinolog

This is an upstream issue:

In [1]: from rasterio.crs import CRS

In [2]: crs = CRS({'init': 'epsg:3857'})

In [3]: crs == 5
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-813cf943d3dc> in <module>()
----> 1 crs == 5

~/git/rasterio/rasterio/_crs.pyx in rasterio._crs._CRS.__eq__()

~/git/rasterio/rasterio/_base.pyx in rasterio._base._osr_from_crs()

/usr/lib/python3.5/collections/__init__.py in __init__(*args, **kwargs)
    975         self.data = {}
    976         if dict is not None:
--> 977             self.update(dict)
    978         if len(kwargs):
    979             self.update(kwargs)

/usr/lib/python3.5/_collections_abc.py in update(*args, **kwds)
    776                     self[key] = other[key]
    777             else:
--> 778                 for key, value in other:
    779                     self[key] = value
    780         for key, value in kwds.items():

TypeError: 'int' object is not iterable

I will open issue there.

drnextgis avatar Aug 14 '18 17:08 drnextgis

Upstream response was "this is expected". Anyway, I would say this is also our fault in the GeoVector.__eq__ method, which only expects another GeoVector:

https://github.com/satellogic/telluric/blob/7c4d75e8f2f5407b5e65436d21339db78ecc61ef/telluric/vectors.py#L431-L438

astrojuanlu avatar Aug 20 '18 06:08 astrojuanlu

This is similar to what attrs does:

http://www.attrs.org/en/stable/why.html#hand-written-classes

astrojuanlu avatar Aug 27 '18 18:08 astrojuanlu

NotImplemented in return means: other side should try to compare, it's the best realization

пн, 27 авг. 2018 г., 20:20 Juan Luis Cano Rodríguez < [email protected]>:

This is similar to what attrs does:

http://www.attrs.org/en/stable/why.html#hand-written-classes

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/satellogic/telluric/issues/123#issuecomment-416319425, or mute the thread https://github.com/notifications/unsubscribe-auth/APSmM--0tN4TFYI2s_yDV1HsDDMpPBffks5uVDhRgaJpZM4V8wY5 .

penguinolog avatar Aug 27 '18 18:08 penguinolog