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

QgsFeatureSource::materialize experiment #5185

Merged
merged 2 commits into from Oct 12, 2017

Conversation

nyalldawson
Copy link
Collaborator

@nyalldawson nyalldawson commented Sep 13, 2017

This PR adds a new "materialize" method to QgsFeatureSource.

When called, materialize takes a QgsFeatureRequest argument and runs it over the source. The resultant features are saved into a new memory provider based QgsVectorLayer, which
is returned by the function (along with ownership of the layer).

This makes it easy to create a new layer from a subset of an existing one.

Materialize also considers subsets of attributes, so that the returned layer only contains fetched fields (and not blank fields filled with NULL values).

The CRS for the output layer will match the input layer, unless setDestinationCrs() has been called with a valid QgsCoordinateReferenceSystem. In this case the output layer will match the destinationCrs() CRS.

The returned layer WKB type will match in the input \a source WKB type, unless the NoGeometry flag is set on this request. In that case the returned layer will not be a spatial layer.

This is designed to (partly - more followups coming from @NathanW2 !) alleviate the horrible pain the comes with having to create a new copy of an existing layer through PyQGIS. Currently you need to do everything yourself - working out the fields, geometry and crs info, wrapping it up in a uri, populating the layer. It's easily ~30 lines of code or more. Ouch!

With materialize you can just do:

new_layer = source_layer.materialize(QgsFeatureRequest().setSubsetOfAttributes([0,4,6]).setFilterExpression('SCORE= \'100\''))
# new_layer is populated and ready to use!
QgsProject.instance().addMapLayer(new_layer)

or

new_layer = source_layer.materialize(QgsFeatureRequest().setDestinationCrs(QgsCoordinateReferenceSystem('epsg:3111'))

or

new_layer = source_layer.materialize(QgsFeatureRequest().setOrderBy(...).setLimit(5))

@m-kuhn
Copy link
Member

m-kuhn commented Sep 13, 2017

Nice one, that's definitely something that's handy to have in a shorthand command.

I'm just wondering if QgsFeatureRequest is the right point to put the code (given that there is also no QgsFeatureRequest::run()).

Could you imagine something like this:

static QgsVectorLayer::fromRequest( source, request, feedback, uri='memory' )

@nyalldawson
Copy link
Collaborator Author

It's quite subjective, but to me a method like

QgsVectorLayer::fromRequest( source, request )

makes me think it will take an exact copy of source, but only with the features from request (i.e. the filtering part alone, not the change in geometry type/fields/CRS). Maybe because it's in QgsVectorLayer and gives the impression of being more about the "layer".

As opposed to a method in QgsFeatureRequest, which seems like a whole new beast, which is tightly linked to the request and its settings.

What was your thinking with the "uri" argument?

@m-kuhn
Copy link
Member

m-kuhn commented Sep 14, 2017

Does QgsVectorLayer::getFeatures( request ) also make you think you'll get an iterator over all features from a layer?

Another thought, what about this?

layer.getFeatures( request ).materialize()

The uri was just some idea, that the output could also be produced in some other format than a memory layer, not sure exactly how that would look.

@nyalldawson
Copy link
Collaborator Author

Does QgsVectorLayer::getFeatures( request ) also make you think you'll get an iterator over all features from a layer?

No, but you expect to get features with the original fields and such intact - just sometimes not fetched. Materialize doesn't do this - if you've only requested some fields, those are ALL that are present in the returned layer. So it's already quite different behavior from how getFeatures() works.

layer.getFeatures( request ).materialize()

I'm reluctant to move it to QgsFeatureIterator - while it might be a semantically better place for it, the materialize command requires full knowledge of the original properties set on the feature request (e.g. the subsetOfAttributes selected). And iterators mess with their request - they add things and remove things and shift parts to requests for the provider and just generally change this request according to their own needs. So that eliminates placing this method in the iterator. (it also brings up questions about how to handle an iterator which may have already iterated through some features!)

So options are leaving it in the request, or moving it to somewhere else like QgsFeatureSource. I still like having it in request as a way of immediately flagging it as "something different" from getFeatures(), but not strongly enough to block this work being merged :)

@m-kuhn
Copy link
Member

m-kuhn commented Sep 15, 2017

QgsFeatureRequest so far is just a "data storage" for the request, without any logic. I would prefer to keep it this way and keep any logic out, wherever that be. Or it suddenly becomes a QObject because we need to signal the progress of the layer creation or similar additions.

When called, materialize takes a QgsFeatureRequest argument
and runs it over the source. The resultant features
are saved into a new memory provider based QgsVectorLayer, which
is returned by the function (along with ownership of the layer)

This makes it easy to create a new layer from a subset of an
existing one.

Materialize also considers subsets of attributes, so that the
returned layer only contains fetched fields (and not blank
fields filled with NULL values).
@nyalldawson nyalldawson changed the title QgsFeatureRequest::materialize experiment QgsFeatureSource::materialize experiment Sep 26, 2017
@nyalldawson
Copy link
Collaborator Author

@m-kuhn I've moved it over to QgsFeatureSource now, does this look better for you?

@m-kuhn
Copy link
Member

m-kuhn commented Sep 26, 2017

Very nice, love it!

Because the world will explode if not
@nyalldawson
Copy link
Collaborator Author

@NathanW2 this approach work for you?

@NathanW2
Copy link
Member

NathanW2 commented Sep 27, 2017 via email

@nyalldawson
Copy link
Collaborator Author

@NathanW2 ping?

@NathanW2
Copy link
Member

+1 looks good to me. Thanks.

@nyalldawson
Copy link
Collaborator Author

Thanks!

@nyalldawson nyalldawson merged commit 0028486 into qgis:master Oct 12, 2017
@nyalldawson nyalldawson deleted the materialize branch October 12, 2017 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants