QEP 407: Introduce a data viewer for the model designer

[ValentinBuira]

This QEP aims to bring a new integrated data viewer directly inside the model designer. This would allow to inspect and debug the state of your model as you build them.

Please see: qep-407-dataviewer-modeldesigner.md

[nyalldawson]

Sounds exciting!

What about the performance issues with the attribute table? Showing an attribute table could potentially block the main thread for a loooooong time. We'd either need to explicitly limit the table to eg the first 200 rows, or make it hidden by default.

[nyalldawson]

Follow up question - how will ownership of layers be handled?

[ValentinBuira]

What about the performance issues with the attribute table? Showing an attribute table could potentially block the main thread for a loooooong time.

We could take inspiration from the preview in the browser panel, the browser panel does two things:

• Only open the table once the toggle is clicked

• And it does limit the number of features to only a 100
  https://github.com/ValentinBuira/QGIS/blob/f7b4ff2ab422c09ca4d38b2bdf6710e8d54fc850/src/gui/qgsbrowserdockwidget_p.cpp#L325

  mAttributeTableFilterModel = new QgsAttributeTableFilterModel( nullptr, tableModel, this );
  tableModel->setRequest( QgsFeatureRequest().setFlags( Qgis::FeatureRequestFlag::NoGeometry ).setLimit( 100 ) );

I like the first one more, because some users will hit the limit whatever limit we set

[gioman]

I like the first one more, because some users will hit the limit whatever limit we set

but we need to avoid the same that happens in db manager... where if by mistake a user clicks the table preview tab and the table is large it gets everything stuck for long time... I think that a limit is really necessary.

[ValentinBuira]

Thanks for raising your concerns on the performance of the attribute table, I resolved myself to add a upper limit to the number of features displayed on the attribute table. I still hope that the limitation would be temporary and that the refactor on the attribute table will bring us batch loading

[ValentinBuira]

Follow up question - how will ownership of layers be handled?

Good point @nyalldawson, quoted from the last revision :

Since the results layers of a previous run is not stable (neither pointer nor layer id). The safer choice is to let the layer store handle the ownership of the temporary layers. And only query the layer using QgsProcessingUtils::mapLayerFromString, the child id and the output parameter name. A more complete example can be found underneath.

Eventually we could clone the layer if we need to modify it. so we have the complete freedom over it.

This would look roughly like this:

QgsModelDesignerDialog::showDataViewerForChildOutput( const QString &childId, const QString &outputParamName )
{

    const QgsProcessingModelChildAlgorithmResult result = mLastResult.childResults().value( childId );
    const QVariantMap childAlgorithmOutputs = result.outputs();
    // Safe guard on childAlgorithmOutputs

    const QVariant output = childAlgorithmOutputs.value( outputParamName);
    // Safe guard on output Variant

    if ( QgsMapLayer *resultLayer = QgsProcessingUtils::mapLayerFromString( output.toString(), mLayerStore ) )
    {
        std::unique_ptr<QgsMapLayer> layer( resultLayer->clone() );
        // Optional have fun with this layer in the data viewer, and don't mess with the original layer in the layer store
    
    }
}

[haubourg]

Thanks for raising your concerns on the performance of the attribute table, I resolved myself to add a upper limit to the number of features displayed on the attribute table. I still hope that the limitation would be temporary and that the refactor on the attribute table will bring us batch loading

Nice proposal, I really miss this from FME. Pagination displaying the first 200 lines is the default in Dbeaver, and I think it is very reasonable. Limiting to 200 lines is not enough, we need pagination for sure.
This could be the default setting for a refactored attribute table in QGIS, I deal with dozens of million lines in most tables, and the only way is to set QGIS to only display selected rows by default.

[ValentinBuira]

Pagination displaying the first 200 lines is the default in Dbeaver, and I think it is very reasonable. Limiting to 200 lines is not enough, we need pagination for sure.

I agree with you I think 200 is not enough and quite low actually, in the latest revision I defaulted to 1500 lines. And above all I added a way to overrided the default throught a setting in the processing settings

[haubourg]

I agree with you I think 200 is not enough and quite low actually, in the latest revision I defaulted to 1500 lines. And above all I added a way to overrided the default throught a setting in the processing settings

Well no, I meant that 200 is perfect if you can show other lines by moving to next page until the end.
Hard limit - 200 or 1500 - is like a random extract of lines. That can be useful to check random issues, but not to inspect all issues
If pagination requires the rewrite of the attribute page, I can totally understand that it is not feasible currently and this shouldn't block your proposal. But as long as we used QEP to discuss user stories, being able to view all lines is mandatory to me

[ValentinBuira]

If pagination requires the rewrite of the attribute page, I can totally understand that it is not feasible currently and this shouldn't block your proposal.

Yes it's outside the scope of this proposal, but make this QEP works with the current limitation of the attribute table was an import part of the proposal

[cbertelli]

I think that one theme is the number of lines. I understand pagination is a problem, but having 50 lines as an example could be sufficient, finding out that the list stops at 200 could be difficult to undestand/accept.
This could be enhanced by a diagnostic tab, finding out duplicates, missing data and other frequent quirks of geographic data.