Skip to content
New issue

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

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

Already on GitHub? # to your account

Support filtering by formation/polygon when calculating statistics #12086

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

jonjenssen
Copy link
Collaborator

@jonjenssen jonjenssen commented Jan 20, 2025

Closes #12057
Closes #12066

@jonjenssen jonjenssen force-pushed the ensemble_formations branch 2 times, most recently from 6050b0e to 4c0f465 Compare January 22, 2025 11:01
@jonjenssen jonjenssen changed the title Support loading formation names when importing an ensemble Support filtering by formation names when calculating statistics Jan 23, 2025
@jonjenssen jonjenssen changed the title Support filtering by formation names when calculating statistics Support filtering by formation/polygon when calculating statistics Jan 27, 2025
@jonjenssen jonjenssen marked this pull request as ready for review January 28, 2025 17:01
Copy link
Member

@magnesj magnesj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well organized and easy to read code. Consider naming improvements.


class RimFormationNames;

class RimFormationTools
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using namespace RimFormationTools instead of class RimFormationTools

{
if ( !polygonCollection->allPolygons().empty() )
{
auto polyGrp = uiOrdering.addNewGroup( "Polygon Selection" );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add skipRemainingFields at end of this function to make sure the polygon selection is not visibible if no polygons are available.

{
cvf::Vec3d pos3D( point );

bool bIncludesCell = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider bool includesCell = false;

RimEclipseCase::initAfterRead();

// handle special formations for ensembles
auto ensemble = firstAncestorOrThisOfType<RimEclipseCaseEnsemble>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider
if ( auto ensemble = firstAncestorOrThisOfType<RimEclipseCaseEnsemble>() )

//--------------------------------------------------------------------------------------------------
std::vector<std::vector<cvf::Vec3d>> RimStatisticsContourMap::selectedPolygons()
{
std::vector<std::vector<cvf::Vec3d>> allLines;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot of text std::vector<std::vector<cvf::Vec3d>
Consider adding a using statement in RigPolyLinesData.

Suggested naming
using PolyLineCoords = std::vector<std::vector<cvf::Vec3d>>;

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
2 participants