Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop V2 suffix on all geometry classes #5491

Merged
merged 17 commits into from Oct 30, 2017

Conversation

Projects
None yet
3 participants
@m-kuhn
Copy link
Member

m-kuhn commented Oct 29, 2017

References qgis/qgis4.0_api#11

This finally brings back a consistent state in naming within the geometry API

Renames

QGIS 2 QGIS 3
QgsPolygon QgsPolygonXY
QgsMultiPoint QgsMultiPointXY
QgsMultiPolyline QgsMultiPolylineXY
QgsMultiPolygon QgsMultiPolygonXY
QgsPolygonV2 QgsPolygon
QgsMultiPointV2 QgsMultiPoint
QgsMultiPolylineV2 QgsMultiPolyline
QgsMultiPolygonV2 QgsMultiPolygon
@NathanW2

This comment has been minimized.

Copy link
Member

NathanW2 commented Oct 29, 2017

@m-kuhn m-kuhn force-pushed the m-kuhn:dropV2 branch from f337162 to 0ddf51b Oct 29, 2017

@m-kuhn m-kuhn added the API Break! label Oct 29, 2017

@m-kuhn

This comment has been minimized.

Copy link
Member Author

m-kuhn commented Oct 29, 2017

@nyalldawson

any idea what this is:

======================================================================
ERROR: testTaskFromFunctionFinishedWithVal (__main__.TestQgsTaskManager)
test that task from function can have callback finished function and is passed result values
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/QGIS/tests/src/python/test_qgstaskmanager.py", line 231, in testTaskFromFunctionFinishedWithVal
    self.assertEqual(finished_single_value_result.value, 5)
AttributeError: 'function' object has no attribute 'value'
----------------------------------------------------------------------

https://travis-ci.org/qgis/QGIS/jobs/294566074#L795-L804

Just seen this the first time (after restarting the job for a different failure). And to be honest, I've got no idea how that code works and how that ever succeeded. A method and a local variable with the same name and an assignment to it to pass back a value... Need to finally get a python book apparently :D

@m-kuhn m-kuhn force-pushed the m-kuhn:dropV2 branch from d6294f8 to 4a5fb70 Oct 29, 2017

@nyalldawson

This comment has been minimized.

Copy link
Contributor

nyalldawson commented Oct 29, 2017

Just seen this the first time (after restarting the job for a different failure). And to be honest, I've got no idea how that code works and how that ever succeeded. A method and a local variable with the same name and an assignment to it to pass back a value... Need to finally get a python book apparently

There's a duplicate line there. But basically it's storing the passed value as a property on the function so that we can later test if that function was called and what value was passed to it. Global variables would also work here, but this keeps the test self-contained.

@nyalldawson

This comment has been minimized.

Copy link
Contributor

nyalldawson commented Oct 29, 2017

Lovely work here!

Can I suggest a related rename? c4f3832 renamed QgsGeometry::fromPolyline to QgsGeometry::fromPolylineXY. I think we should do this for all these methods if we're renaming the related classes. E.g. fromMultiPoint -> fromMultiPointXY, fromPolygon -> fromPolygonXY, etc. Then we've got scope to later in 3.x readd new fromMultiPoint/fromPolygon etc which operate on QgsPoint.

@m-kuhn

This comment has been minimized.

Copy link
Member Author

m-kuhn commented Oct 30, 2017

There's a duplicate line there. But basically it's storing the passed value as a property on the function so that we can later test if that function was called and what value was passed to it. Global variables would also work here, but this keeps the test self-contained.

Thanks. In the end, I figured it out which led me to propose #5495 which should make things even more self-contained.

@m-kuhn m-kuhn changed the title Drop V2 Prefix on all geometry classes Drop V2 suffix on all geometry classes Oct 30, 2017

@m-kuhn m-kuhn force-pushed the m-kuhn:dropV2 branch from 6852971 to a9340d0 Oct 30, 2017

m-kuhn added some commits Oct 29, 2017

Rename from[GeometryType] to from[GeometryType]XY
- QgsGeometry::fromPoint() was renamed to fromPointXY()
- QgsGeometry::fromMultiPoint() was renamed to fromMultiPointXY()
- QgsGeometry::fromMultiPolyline() was renamed to fromMultiPolylineXY()
- QgsGeometry::fromPolygon() was renamed to fromPolygonXY()
- QgsGeometry::fromMultiPolygon() was renamed to fromMultiPolygonXY()
Rename QgsGeometry::fromPoint to QgsGeometry::fromPointXY
Also introduces the from[Geometry]XY for QgsGeometryFactory

@m-kuhn m-kuhn force-pushed the m-kuhn:dropV2 branch from 485d727 to d2c1011 Oct 30, 2017

@m-kuhn m-kuhn merged commit 1d21072 into qgis:master Oct 30, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@NathanW2

This comment has been minimized.

Copy link
Member

NathanW2 commented Oct 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.