Skip to content

Conversation

@sgillies
Copy link
Member

Resolves #1379

@sgillies sgillies added the bug label Apr 21, 2024
@sgillies sgillies added this to the 1.10 milestone Apr 21, 2024
@sgillies sgillies self-assigned this Apr 21, 2024
fiona/model.py Outdated
from fiona.errors import FionaDeprecationWarning

_coord_repr = reprlib.Repr()
_coord_repr.maxlist = 1
Copy link
Member Author

Choose a reason for hiding this comment

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

For real-world features, the coordinates are likely to make up most of the representation. Showing 20+ can easily overflow displays. One is enough to see where the feature is in space, roughly. 6 would be reprlib's default, but that's still a lot of characters if we're dealing with small scale, projected features.

geometry=Geometry(type="LineString", coordinates=[(0, 0)] * 100),
properties=Properties(a=1, foo="bar"),
)
assert repr(feat) == "fiona.Feature(geometry=fiona.Geometry(coordinates=[(0, 0), ...], type='LineString'), id='1', properties=fiona.Properties(a=1, foo='bar'))"
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of Fiona 1.8's GeoJSON representation, I propose this. It shows which classes are involved, which is nice. And in a form that could be used to reproduce the objects, modulo the coordinates truncation.

@sgillies sgillies requested a review from mwtoews April 21, 2024 20:14
@sgillies
Copy link
Member Author

@loicdtx @mwtoews what do you think of this?

Copy link
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

Useful reprs are welcome! (And reprlib has evaded my attention!)

As a suggestion, you could code less by adding this once to Object around L179:

    def __repr__(self):
        kvs = [f"{k}={v!r}" for k, v in self.items()]
        return "{}({})".format(self.__class__.__name__, ", ".join(kvs))

but keep Geometry.__repr__ for the _coord_repr difference.

Also also suggest to see the repr class name with the shorter Geometry rather than fiona.Geometry. A counterpoint to this suggestion is that it may confuse users with other library object names also called "Geometry" (I actually can't name any right now, but certainly they exist).

@sgillies
Copy link
Member Author

@mwtoews thank you! I'd like to keep the fiona package name in there for the sake of disambiguation, thinking that in some cases the people that see these will not have intentionally imported fiona, but are getting it second-hand.

As to moving the use of reprlib.repr() to the base class, I found that it, by default, shortens representations too much. I'm finding it useful for shortening representation of geometry coordinates. It might be useful for shortening very long mappings of properties... I'll try that out.

@mwtoews
Copy link
Member

mwtoews commented Apr 21, 2024

@sgillies it's fine to keep fiona. in the repr, so my suggestion would only need to adjust with return "fiona.{}({})".format(self.__class__.__name__, ", ".join(kvs)).

As for shortening other instances of Object, I wasn't suggesting this. But I suppose there could be instances of datasets with many fields or with individual fields with "large" blobs or text attributes.

@sgillies
Copy link
Member Author

@mwtoews thank you for the suggestions!

mwtoews
mwtoews previously approved these changes Apr 22, 2024
@loicdtx
Copy link
Contributor

loicdtx commented Apr 22, 2024

Thanks @sgillies , that's definitely an improvement. I tested it on tests/data/gre.shp and it looks great overall. Two minor observations:

  • pprint doesn't work unless coerced to dict (not saying it should work, just pointing it out)
  • dict coercion only happens on top level, so that pprint(dict(feature)) does not pprint the properties (that's probably another "issue" though)
>>> import fiona
>>> from pprint import pprint
>>> fc = fiona.open('tests/data/gre.shp')
>>> fc[0]
fiona.Feature(geometry=fiona.Geometry(coordinates=[[(-61.173214300000005, 12.516654800000001), ...]], type='Polygon'), id='0', properties=fiona.Properties(flag='http://upload.wikimedia.org/wikipedia/commons/b/bc/Flag_of_Grenada.svg', name='Grenada', name_cs='Grenada', name_de='Grenada', name_en='Grenada', name_eo='Grenado', name_fr='Grenade', name_fy='Grenada', name_hr='Grenada', name_nl='Grenada', name_ru='Гренада', name_sl='Grenada', name_ta='கிரெனடா', name_uk='Гренада', boundary='administrative', name_tzl='Grenada', timezone='America/Grenada', wikidata='Q769', ISO3166-1='GD', wikipedia='en:Grenada', admin_leve='2', is_in_cont='North America', ISO3166-1_='GD', ISO3166-_1='GRD', ISO3166-_2='308'))

>>> pprint(fc[0])
fiona.Feature(geometry=fiona.Geometry(coordinates=[[(-61.173214300000005, 12.516654800000001), ...]], type='Polygon'), id='0', properties=fiona.Properties(flag='http://upload.wikimedia.org/wikipedia/commons/b/bc/Flag_of_Grenada.svg', name='Grenada', name_cs='Grenada', name_de='Grenada', name_en='Grenada', name_eo='Grenado', name_fr='Grenade', name_fy='Grenada', name_hr='Grenada', name_nl='Grenada', name_ru='Гренада', name_sl='Grenada', name_ta='கிரெனடா', name_uk='Гренада', boundary='administrative', name_tzl='Grenada', timezone='America/Grenada', wikidata='Q769', ISO3166-1='GD', wikipedia='en:Grenada', admin_leve='2', is_in_cont='North America', ISO3166-1_='GD', ISO3166-_1='GRD', ISO3166-_2='308'))

>>> pprint(dict(fc[0]['properties']))
{'ISO3166-1': 'GD',
 'ISO3166-1_': 'GD',
 'ISO3166-_1': 'GRD',
 'ISO3166-_2': '308',
 'admin_leve': '2',
 'boundary': 'administrative',
 'flag': 'http://upload.wikimedia.org/wikipedia/commons/b/bc/Flag_of_Grenada.svg',
 'is_in_cont': 'North America',
 'name': 'Grenada',
 'name_cs': 'Grenada',
 'name_de': 'Grenada',
 'name_en': 'Grenada',
 'name_eo': 'Grenado',
 'name_fr': 'Grenade',
 'name_fy': 'Grenada',
 'name_hr': 'Grenada',
 'name_nl': 'Grenada',
 'name_ru': 'Гренада',
 'name_sl': 'Grenada',
 'name_ta': 'கிரெனடா',
 'name_tzl': 'Grenada',
 'name_uk': 'Гренада',
 'timezone': 'America/Grenada',
 'wikidata': 'Q769',
 'wikipedia': 'en:Grenada'}

>>> pprint(dict(fc[0]))
{'geometry': fiona.Geometry(coordinates=[[(-61.173214300000005, 12.516654800000001), ...]], type='Polygon'),
 'id': '0',
 'properties': fiona.Properties(flag='http://upload.wikimedia.org/wikipedia/commons/b/bc/Flag_of_Grenada.svg', name='Grenada', name_cs='Grenada', name_de='Grenada', name_en='Grenada', name_eo='Grenado', name_fr='Grenade', name_fy='Grenada', name_hr='Grenada', name_nl='Grenada', name_ru='Гренада', name_sl='Grenada', name_ta='கிரெனடா', name_uk='Гренада', boundary='administrative', name_tzl='Grenada', timezone='America/Grenada', wikidata='Q769', ISO3166-1='GD', wikipedia='en:Grenada', admin_leve='2', is_in_cont='North America', ISO3166-1_='GD', ISO3166-_1='GRD', ISO3166-_2='308')}

@sgillies
Copy link
Member Author

@loicdtx thank you for pointing out the issue with dict(). I've addressed that in the latest commits, you'll now get dicts all the way down.

@sgillies sgillies merged commit 628e781 into main Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make features printable again

4 participants