Re: [gdal-dev] Use of C++ iterators in API
On 10 April 2018 at 23:06, Even Rouault wrote: > On mardi 10 avril 2018 22:34:56 CEST Mateusz Loskot wrote: >> On 10 April 2018 at 22:06, Even Rouault wrote: >> > >> > Just wanted to advertize the recent new introduction of C++ iterators >> > (...) >> >> Good one, thanks! >> >> > for( auto&& poLayer: poDS->GetLayers() ) >> >> Do the iterators return a proxy reference hence the tutorial prefers >> auto&& over plain auto&? >> (ie. like std::vector iterator does) > > Not completely sure to understand what a proxy reference is (C++11 stuff is > still very new to me, and I don't pretend to master C++98, so ...), but I feel > that must be close to what I have used in some cases, particularly wen > iterating over fields of a features, or points of a linestring. I didn't look at the iterators impl. yet. I just looked at your example and wondered if there are proxies - for GDAL/OGR, those could be necessary, I imagine. I mean a proxy as an object that 'looks' like a reference. It's often necessary to for (proxied) collections of objects that can not be accessed directly, via regular pointers/references. > I read in some tutorial that using auto&& worked in all situations, including > when the returned reference was const, so now I just use that everywhere. auto&& is most useful when container::reference returns a proxy object, which would not bind to auto& (it would bind to auto const& though). > In fact the nature of the object which is returned depends on the iterator, > and as documented in the API, for the sake of implementation simplicity, those > iterators do not necessarily respect the whole semantics of iterators. > > That is if you do > > auto iter = poLineString->begin(); > OGRPoint& oPoint = *iter; > ++iter; > OGRPoint& oAnotherPoint = *iter; > In fact &oAnotherPoint == &oPoint. At first, it seems like they model the input iterators, single-pass iterator category http://en.cppreference.com/w/cpp/concept/InputIterator > So basically you should only use them in range based loops, where you don't > really manipulate the iterator and cannot do stuff like the above. Or, use like any input iterators, seems safe. > I guess the "correct" solution would have been to return a OGRPointProxy that > would keep a link to the linestring and the point index. But not sure how you > would do that for a const iterator where the operator * is supposed to > returned a const reference... Or reference-like proxy like std::vector does. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
On mardi 10 avril 2018 22:34:56 CEST Mateusz Loskot wrote: > On 10 April 2018 at 22:06, Even Rouault wrote: > > Hi, > > > > Just wanted to advertize the recent new introduction of C++ iterators > > (...) > > Good one, thanks! > > > for( auto&& poLayer: poDS->GetLayers() ) > > Do the iterators return a proxy reference hence the tutorial prefers > auto&& over plain auto&? > (ie. like std::vector iterator does) Not completely sure to understand what a proxy reference is (C++11 stuff is still very new to me, and I don't pretend to master C++98, so ...), but I feel that must be close to what I have used in some cases, particularly wen iterating over fields of a features, or points of a linestring. I read in some tutorial that using auto&& worked in all situations, including when the returned reference was const, so now I just use that everywhere. In fact the nature of the object which is returned depends on the iterator, and as documented in the API, for the sake of implementation simplicity, those iterators do not necessarily respect the whole semantics of iterators. That is if you do auto iter = poLineString->begin(); OGRPoint& oPoint = *iter; ++iter; OGRPoint& oAnotherPoint = *iter; In fact &oAnotherPoint == &oPoint. The reason is that internally a line string is not an array of OGRPoint, but an array of OGRRawPoint + optional array of z + optional array of m, so the iterator object only holds a single OGRPoint and always return the reference to it when you dereference it, and updates its content when you increment it. So basically you should only use them in range based loops, where you don't really manipulate the iterator and cannot do stuff like the above. I guess the "correct" solution would have been to return a OGRPointProxy that would keep a link to the linestring and the point index. But not sure how you would do that for a const iterator where the operator * is supposed to returned a const reference... Except allocating a std::array in the iterator, which seems overkill. Improvements (if doable and not adding (too much) overhead to the underlying objects) are welcome of course. Even -- Spatialys - Geospatial professional services http://www.spatialys.com ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] Use of C++ iterators in API
On 10 April 2018 at 22:06, Even Rouault wrote: > Hi, > > Just wanted to advertize the recent new introduction of C++ iterators (...) Good one, thanks! > for( auto&& poLayer: poDS->GetLayers() ) Do the iterators return a proxy reference hence the tutorial prefers auto&& over plain auto&? (ie. like std::vector iterator does) Best regards, -- Mateusz Loskot, http://mateusz.loskot.net ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
[gdal-dev] Use of C++ iterators in API
Hi,
Just wanted to advertize the recent new introduction of C++ iterators over
GDALDataset (to iterate over its bands, layers and features), OGRLayer (to
iterate overs its features), OGRFeature (to iterate over its field values) and
OGRGeometry (if you iterate over a OGRGeometryCollection, you get its members.
If you iterate over a OGRPolygon, you get its rings, etc...)
Fields can also be get and set using the [] operator on a feature.
Taken from the updated API tutorial : http://gdal.org/ogr_apitut.html
#include "ogrsf_frmts.h"
int main()
{
GDALAllRegister();
GDALDatasetUniquePtr poDS(GDALDataset::Open( "point.shp",
GDAL_OF_VECTOR));
if( poDS == nullptr )
{
printf( "Open failed.\n" );
exit( 1 );
}
for( auto&& poLayer: poDS->GetLayers() )
{
for( auto&& poFeature: *poLayer )
{
for( auto&& oField: *poFeature )
{
switch( oField.GetType() )
{
case OFTInteger:
printf( "%d,", oField.GetInteger() );
break;
case OFTInteger64:
printf( CPL_FRMT_GIB ",", oField.GetInteger64() );
break;
case OFTReal:
printf( "%.3f,", oField.GetDouble() );
break;
case OFTString:
printf( "%s,", oField.GetString() );
break;
default:
printf( "%s,", oField.GetAsString() );
break;
}
}
}
}
There's also a visitor over OGRGeometry subclasses that can make life easier
( http://gdal.org/classOGRDefaultConstGeometryVisitor.html )
The autotest cpp stuff starting at
https://github.com/OSGeo/gdal/blob/master/autotest/cpp/test_ogr.cpp#L963
should get a good picture of all the new API.
Even
--
Spatialys - Geospatial professional services
http://www.spatialys.com
___
gdal-dev mailing list
gdal-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] PEP8 compliance for Python code
On 10/04/18 19:05, Even Rouault wrote: > Apparently autopep8 can fix most of issues, except line too longs ? I think that's right. I would not plan to touch long lines. The worst of them can be fixed over time when other edits are taking place. > I guess your plans do not cover the SWIG generated .py files, right ? Right. We'll just skip those files. Cheers, Ben ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
Re: [gdal-dev] PEP8 compliance for Python code
Ben, Thanks for leading this. Apparently autopep8 can fix most of issues, except line too longs ? Do you plan to fix even that ? That could be quite painful now, and in the future, as in the testing code, you compare things like WKT strings that can be long, and that would be a burden to make sure we split them to 80 columns. I guess your plans do not cover the SWIG generated .py files, right ? Even > The Python code in the GDAL tree covers a few things including the > testsuite and a Python module for GDAL itself. There is quite a lot of > this code and the coding conventions are a bit all over the place. There > are several deprecated idioms still being used. I think we should > settle on the Python PEP8 coding style guidelines as a way of > standardising the appearance of the code across the tree. > > I have so far made a small number of changes detected with 'flake8', but > some of the changes (such as removing extraneous spaces around > parentheses) are quite invasive and should be done more carefully. > > I propose to use the 'autopep8' tool to automatically fix all of the > trivial problems to do with whitespace. This is much faster than manual > editing and reduces the risk of errors. I plan to inspect the diffs and > run 'git diff -w' to double check that there are no non-whitespace > changes. The changes would be made one warning category at a time, on > cascading (serial) git branches so that there will be minimal merge > conflicts. Having a number of parallel branches is just asking for many > merge conflicts. > > At the end of all this, we can enable flake8 support in Travis CI to > maintain compliance with the coding standard. > > Thoughts? > > Cheers, Ben > ___ > gdal-dev mailing list > gdal-dev@lists.osgeo.org > https://lists.osgeo.org/mailman/listinfo/gdal-dev -- Spatialys - Geospatial professional services http://www.spatialys.com ___ gdal-dev mailing list gdal-dev@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/gdal-dev
