[order] 2: read_wkt to read

[Barend]

Hi,

Yesterday I mailed part 1 of the order of parameters (combine). This is
about read_wkt.

It's signature: void read_wkt(string, Geometry);

should probably change to read_wkt(Geometry, string) to conform to the
order GeometryIn, GeometryOut, others.

At the same time, read_wkt is currently an extension. It is extensively
used in nearly every testcase, many samples, etc. If we go to the
release-branche, we either should skip all extensions or move WKT to
something else.

To allow for all these changes at the same time, I propose the following.

1) We make a new folder "domains" which is subdivided as the
"extensions" are. So we now get a:
domains/gis/io/wkt for the WKT-parser.

2) We move wkt to there

3) At the higher level (in io, or in domains, or in algorithms) we make
one new algorithm called "read". This one has a required template
parameter denoting the format. And it reverses the arguments. So we get,
for example:

polygon_2d triangle;
boost::geometry::read<format_wkt>(triangle, "POLYGON((1 1,1 2,2 1,1 1))");

and we can get the same for format_wkb and possibly other formats we
will support for reading. There even might be a format_autodetect which
figures out the format from the string. In OGC this algorithm is called
geomfromtext, returning a geometry. I had this earlier but it was
discarded because there are so many text formats, but now we might
consider using that name again.

Changing all read_wkt to read<format_wkt> will be a bigger change
especially for me, because of all tests (reversing the arguments), but I
think it is manageable, and better like this. The read_wkt function
itself can stay in extensions for backwards compatiblity, with a
compiler warning that it is deprecated.

There is one tweak, because there is also:
read_wkt(std::string const& wkt, OutputIterator out)
only working for linestring and ring. This case is probably not used and
might be deprecated either (so no new function read<format_wkt> for it).

Objections to this all?

Regards, Barend


--
Barend Gehrels
http://about.me/barendgehrels

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl

[Mateusz Loskot]

On 27 February 2011 11:49, Barend Gehrels <[hidden email]> wrote:
> Hi,
>
> Yesterday I mailed part 1 of the order of parameters (combine). This is
> about read_wkt.
>
> It's signature: void read_wkt(string, Geometry);
>
> should probably change to read_wkt(Geometry, string) to conform to the order
> GeometryIn, GeometryOut, others.

More precisely, you mean read_wkt(Geometry& out, string const& in), right?

OK. I also prefer input/output argument as first.
It fits the tradition of std::memcpy and others.

> At the same time, read_wkt is currently an extension. It is extensively used
> in nearly every testcase, many samples, etc. If we go to the
> release-branche, we either should skip all extensions or move WKT to
> something else.

I have proposed
geometry/io/
presumably with substructure of
geometry/io/write_dsv.hpp
geometry/io/domain/gis/read_wkt.hpp
...

> To allow for all these changes at the same time, I propose the following.
>
> 1) We make a new folder "domains" which is subdivided as the "extensions"
> are. So we now get a:
> domains/gis/io/wkt for the WKT-parser.

OK.

Where write_dsv.hpp fits?
Where SVG I/O utilities would fit?

> 2) We move wkt to there
>
> 3) At the higher level (in io, or in domains, or in algorithms) we make one
> new algorithm called "read". This one has a required template parameter
> denoting the format. And it reverses the arguments. So we get, for example:
>
> polygon_2d triangle;
> boost::geometry::read<format_wkt>(triangle, "POLYGON((1 1,1 2,2 1,1 1))");

I like it!

> and we can get the same for format_wkb and possibly other formats we will
> support for reading. There even might be a format_autodetect which figures
> out the format from the string. In OGC this algorithm is called
> geomfromtext, returning a geometry. I had this earlier but it was discarded
> because there are so many text formats, but now we might consider using that
> name again.

Yes, it opens plenty of design possibilities to users indeed.

> Changing all read_wkt to read<format_wkt> will be a bigger change especially
> for me, because of all tests (reversing the arguments), but I think it is
> manageable, and better like this. The read_wkt function itself can stay in
> extensions for backwards compatiblity, with a compiler warning that it is
> deprecated.

OK. Let me know if you need help.

> There is one tweak, because there is also:
> read_wkt(std::string const& wkt, OutputIterator out)
> only working for linestring and ring. This case is probably not used and
> might be deprecated either (so no new function read<format_wkt> for it).
>
> Objections to this all?


No objections from me.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
Member of ACCU, http://accu.org
_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl

[Mateusz Loskot]

2011/10/19 Mateusz Łoskot <[hidden email]>:

> On 27 February 2011 11:49, Barend Gehrels <[hidden email]> wrote:
>> Yesterday I mailed part 1 of the order of parameters (combine). This is
>> about read_wkt.
>>
>> It's signature: void read_wkt(string, Geometry);
>>
>> should probably change to read_wkt(Geometry, string) to conform to the order
>> GeometryIn, GeometryOut, others.
>
> More precisely, you mean read_wkt(Geometry& out, string const& in), right?
>
> OK. I also prefer input/output argument as first.
> It fits the tradition of std::memcpy and others.
>
>> At the same time, read_wkt is currently an extension. It is extensively used
>> in nearly every testcase, many samples, etc. If we go to the
>> release-branche, we either should skip all extensions or move WKT to
>> something else.
>
> I have proposed
> geometry/io/
> presumably with substructure of
> geometry/io/write_dsv.hpp
> geometry/io/domain/gis/read_wkt.hpp
> ...
>
>> To allow for all these changes at the same time, I propose the following.
>>
>> 1) We make a new folder "domains" which is subdivided as the "extensions"
>> are. So we now get a:
>> domains/gis/io/wkt for the WKT-parser.
>
> OK.
>
> Where write_dsv.hpp fits?
> Where SVG I/O utilities would fit?
>
>> 2) We move wkt to there
>>
>> 3) At the higher level (in io, or in domains, or in algorithms) we make one
>> new algorithm called "read". This one has a required template parameter
>> denoting the format. And it reverses the arguments. So we get, for example:
>>
>> polygon_2d triangle;
>> boost::geometry::read<format_wkt>(triangle, "POLYGON((1 1,1 2,2 1,1 1))");
>
> I like it!
>
>> and we can get the same for format_wkb and possibly other formats we will
>> support for reading. There even might be a format_autodetect which figures
>> out the format from the string. In OGC this algorithm is called
>> geomfromtext, returning a geometry. I had this earlier but it was discarded
>> because there are so many text formats, but now we might consider using that
>> name again.
>
> Yes, it opens plenty of design possibilities to users indeed.
>
>> Changing all read_wkt to read<format_wkt> will be a bigger change especially
>> for me, because of all tests (reversing the arguments), but I think it is
>> manageable, and better like this. The read_wkt function itself can stay in
>> extensions for backwards compatiblity, with a compiler warning that it is
>> deprecated.
>
> OK. Let me know if you need help.
>
>> There is one tweak, because there is also:
>> read_wkt(std::string const& wkt, OutputIterator out)
>> only working for linestring and ring. This case is probably not used and
>> might be deprecated either (so no new function read<format_wkt> for it).
>>
>> Objections to this all?
>
>
> No objections from me.

I have no idea how and why, but I didn't realise this post is from
February 2011.
I have been doing big migration of my e-mail server and I somehow
found this old post on top of my inbox.

If the matter is still valid, let's continue :)
Otherwise, please ignore it.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
Member of ACCU, http://accu.org
_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl

[Barend]

Hi Mateusz,

Yes, this answer on the old mail is still relevant.

On 19-10-2011 15:51, Mateusz Łoskot wrote:

> On 27 February 2011 11:49, Barend Gehrels<[hidden email]>  wrote:
>> Hi,
>>
>> Yesterday I mailed part 1 of the order of parameters (combine). This is
>> about read_wkt.
>>
>> It's signature: void read_wkt(string, Geometry);
>>
>> should probably change to read_wkt(Geometry, string) to conform to the order
>> GeometryIn, GeometryOut, others.
> More precisely, you mean read_wkt(Geometry&  out, string const&  in), right?

Yes.

>
> OK. I also prefer input/output argument as first.
> It fits the tradition of std::memcpy and others.
>
>> At the same time, read_wkt is currently an extension. It is extensively used
>> in nearly every testcase, many samples, etc. If we go to the
>> release-branche, we either should skip all extensions or move WKT to
>> something else.
> I have proposed
> geometry/io/
> presumably with substructure of
> geometry/io/write_dsv.hpp
> geometry/io/domain/gis/read_wkt.hpp
> ...
>
>> To allow for all these changes at the same time, I propose the following.
>>
>> 1) We make a new folder "domains" which is subdivided as the "extensions"
>> are. So we now get a:
>> domains/gis/io/wkt for the WKT-parser.
> OK.
>
> Where write_dsv.hpp fits?
> Where SVG I/O utilities would fit?
>
>> 2) We move wkt to there
>>
>> 3) At the higher level (in io, or in domains, or in algorithms) we make one
>> new algorithm called "read". This one has a required template parameter
>> denoting the format. And it reverses the arguments. So we get, for example:
>>
>> polygon_2d triangle;
>> boost::geometry::read<format_wkt>(triangle, "POLYGON((1 1,1 2,2 1,1 1))");
> I like it!

Right. It is implemented as such and forwards to the normal
implementation. Because WKT is heavily used in the unit tests, I did not
have the time to modify them all, so the unit tests use the old
implementation (I just checked and there are no unit test for the new
read method, that at least should be done...).


>
>> Changing all read_wkt to read<format_wkt>  will be a bigger change especially
>> for me, because of all tests (reversing the arguments), but I think it is
>> manageable, and better like this. The read_wkt function itself can stay in
>> extensions for backwards compatiblity, with a compiler warning that it is
>> deprecated.
> OK. Let me know if you need help.

So yes ;-)
That compiler warning is not implemented yet (of course).

Regards, Barend

_______________________________________________
ggl mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/ggl