app enum improvements

[magnesj]

PR Type

Enhancement

---

Description

• Refactors AppEnum to use template-based mapper class

• Adds support for std::optional PDM fields

• Enhances Python API with key-value store operations

• Improves well path and grid property extraction capabilities

---

Diagram Walkthrough

flowchart LR
  A["AppEnum"] -- "refactored with" --> B["AppEnumMapper<T>"]
  C["PDM Fields"] -- "enhanced with" --> D["std::optional<T>"]
  E["Python API"] -- "extended with" --> F["Key-Value Store"]
  G["Well Path"] -- "enhanced with" --> H["Property Extraction"]
  I["Grid Case"] -- "enhanced with" --> J["Position Sampling"]

Loading

File Walkthrough

	Relevant files
Enhancement	30 files cafAppEnum.h Refactor AppEnum to use template mapper                                    +20/-188 cafAppEnumMapper.h Add new template-based enum mapper class                                  +230/-0  cafInternalPdmFieldTypeSpecializations.h Add specialization for std::optional fields                            +59/-20  cafPdmFieldScriptingCapability.h Add scripting support for optional fields                                +53/-0    cafInternalPdmStreamOperators.h Add stream operators for std::optional                                      +40/-0    RimcWellPath.cpp Add well path property extraction method                                  +97/-2    RimcWellPath.h Add well path property extraction interface                            +23/-0    RimcEclipseCase.cpp Add grid property sampling for positions                                  +113/-7  RimcEclipseCase.h Add grid property sampling interface                                          +23/-0    RigDoglegTools.cpp Add dogleg calculation utilities                                                  +66/-0    RigDoglegTools.h Add dogleg calculation interface                                                  +36/-0    RiaGrpcKeyValueStoreService.cpp Add get and remove operations                                                        +104/-28 RiaGrpcKeyValueStoreService.h Add get and remove method declarations                                      +9/-0      RiaKeyValueStoreUtil.cpp Add byte vector conversion utility                                              +19/-0    RiaKeyValueStoreUtil.h Add byte vector conversion interface                                          +1/-0      project.py Add key-value store Python methods                                              +48/-0    well_path.py Add trajectory properties extraction                                          +62/-0    case.py Add grid property sampling method                                                +54/-0    RimcProject.cpp Add well path collection accessor                                                +35/-0    RimcProject.h Add well path collection interface                                              +14/-0    RimcWellPathCollection.cpp Add well path import method                                                            +79/-0    RimcWellPathCollection.h Add well path import interface                                                      +42/-0    RimWellPathCollection.cpp Allow multiple imports of same well                                            +19/-17  RimWellPath.cpp Expose well path name field                                                            +2/-3      RicImportGeneralDataFeature.cpp Add ESMRY file support to dialogs                                                +17/-1    RicRecursiveFileSearchDialog.cpp Add ESMRY file type support                                                            +5/-0      RicRecursiveFileSearchDialog.h Add ESMRY file type enum                                                                  +3/-0      RifOpmSummaryTools.cpp Enhance realization number extraction                                        +3/-2      RiaEnsembleNameTools.cpp Detect single ensemble for Everest grouping                            +7/-0      KeyValueStore.proto Add get and remove service methods                                              +9/-0
Bug fix	4 files cafPdmPythonGenerator.cpp Handle empty strings in optional fields                                    +24/-3    RicImportSummaryCasesFeature.cpp Fix logic error and add ESMRY support                                        +4/-1      RimSummaryCurve.cpp Add project version check for axis                                              +1/-1      RiaProjectFileTools.cpp Fix version comparison for undefined minors                            +5/-2
Tests	8 files OptionalFields.cpp Add optional fields test object                                                    +25/-0    OptionalFields.h Add optional fields test interface                                              +17/-0    AggregatedTypesInField.cpp Add optional field unit tests                                                        +14/-2    cafPdmFieldSerializationTest.cpp Add optional field serialization tests                                      +57/-0    test_key_value_store.py Add key-value store Python tests                                                  +47/-0    test_cases.py Add grid property sampling tests                                                  +130/-0  test_wells.py Add trajectory properties tests                                                    +26/-0    RifCaseRealizationParametersReader-Test.cpp Add underscore separator test                                                        +14/-5
Miscellaneous	1 files ResInsightVersion.cmake Bump development version to .08                                                    +1/-1
Configuration changes	1 files AppFwkUnitTest.yml Update workflow to Windows 2025                                                    +1/-1
Additional files	21 files RimModeledWellPath.cpp +0/-16    RimModeledWellPath.h +0/-1      RimOsduWellPath.cpp +0/-6      CMakeLists_files.cmake +2/-0      CMakeLists_files.cmake +2/-0      cafMockObjects.cpp +2/-0      cafMockObjects.h +3/-2      CMakeLists.txt +1/-0      cafPdmUiFieldSpecialization.h +5/-11    CMakeLists.txt +2/-0      MainWindow.cpp +2/-0      create_intersection.py +1/-1      export_well_path_trajectory_with_properties.py +61/-0    fishbones_completion.py +1/-1      modeled_well_path.py +1/-1      modeled_well_path_lateral.py +1/-1      __init__.py +1/-0      mypy.ini +3/-0      test_create_well_path.py +3/-3      test_object_lifetime.py +1/-1      test_wells_path_completions.py +3/-3

---

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue Dogleg angle formula uses cos(inc2 - inc1) - sin(inc1)sin(inc2)(1 - cos(azi2 - azi1)), which differs from the standard minimum curvature formula cos(DL) = cos inc1 cos inc2 + sin inc1 sin inc2 cos(Δazi). Verify correctness against expected dogleg computation and unit-test with known trajectories. // Dogleg angle using spherical trigonometry (clamp to avoid numerical instability) double cosDogleg = std::clamp( std::cos( inc2 - inc1 ) - std::sin( inc1 ) * std::sin( inc2 ) * ( 1 - std::cos( azi2 - azi1 ) ), -1.0, 1.0 ); return cvf::Math::toDegrees( std::acos( cosDogleg ) ); } Edge Cases Exporting values converts coordinates to cvf::Vec3d(xs[i], ys[i], -zs[i]); confirm Z sign convention matches caller (Python API provides positive depth). Also ensure timeStep bounds check occurs before creating/accessing results to avoid unnecessary loads. for ( size_t i = 0; i < xs.size(); i++ ) { positions.push_back( cvf::Vec3d( xs[i], ys[i], -zs[i] ) ); } std::vector<float> values; for ( const cvf::Vec3d& position : positions ) { auto cellIdx = mainGrid->findReservoirCellIndexFromPoint( position ); if ( cellIdx != cvf::UNDEFINED_SIZE_T ) { float valueFromEclipse = static_cast<float>( resultAccessor->cellScalar( cellIdx ) ); values.push_back( valueFromEclipse ); } Optional Parsing std::optional UI specialization treats empty QString (after quote removal) as nullopt; ensure this does not erroneously nullify valid string values like "0" or "false". Add tests for optional numeric zero and empty-string semantics where empty string should mean None only for strings. { public: /// Convert the field value into a QVariant static QVariant convert( const std::optional<T>& value ) { if ( value.has_value() ) { return PdmValueFieldSpecialization<T>::convert( value.value() ); } return QVariant(); } /// Set the field value from a QVariant static void setFromVariant( const QVariant& variantValue, std::optional<T>& value ) { // An empty QVariant means no value, and we should set the optional to std::nullopt auto stringText = variantValue.toString(); stringText.remove( '"' ); if ( stringText.isEmpty() ) { value.reset(); return; } T valueOfType; PdmValueFieldSpecialization<T>::setFromVariant( variantValue, valueOfType ); value = valueOfType; } static bool isDataElementEqual( const QVariant& variantValue, const QVariant& variantValue2 ) { return variantValue == variantValue2; } /// Methods to get a list of options for a field, specialized for std::optional<T> static QList<PdmOptionItemInfo> valueOptions( QString keyword, const std::optional<T>& ) { QList<PdmOptionItemInfo> optionList; return optionList; } /// Methods to retrieve the possible PdmObject pointed to by a field static void childObjects( const PdmDataValueField<std::optional<T>>& field, std::vector<PdmObjectHandle*>* objects ) {

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Risky enum refactor The AppEnum refactor splits logic into a new AppEnumMapper but cafAppEnum.h still references AppEnumMapper without consistently including or namespace-qualifying it, which can cause ODR or include-order dependent build/runtime issues across modules. Ensure all AppEnum uses consistently include cafAppEnumMapper.h, fix any missing qualifications, and validate that generated Python/type-mapping code relies on the new mapper API uniformly to avoid subtle regressions. Examples: Fwk/AppFwk/cafProjectDataModel/cafPdmCore/cafAppEnum.h [170] static QStringList uiTexts() { return AppEnumMapper::instance()->uiTexts(); } Fwk/AppFwk/cafProjectDataModel/cafPdmCore/cafAppEnum.h [177-182] static T fromText( const QString& text ) { T val; AppEnumMapper::instance()->enumVal( val, text ); return val; } Solution Walkthrough: Before: template <class T> class AppEnum : public AppEnumInterface { public: // ... static QStringList uiTexts() { return AppEnumMapper::instance()->uiTexts(); } static T fromText( const QString& text ) { T val; AppEnumMapper::instance()->enumVal( val, text ); return val; } // ... }; After: template <class T> class AppEnum : public AppEnumInterface { public: // ... static QStringList uiTexts() { return AppEnumMapper<T>::instance()->uiTexts(); } static T fromText( const QString& text ) { T val; AppEnumMapper<T>::instance()->enumVal( val, text ); return val; } // ... }; Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies critical compilation errors in cafAppEnum.h where AppEnumMapper is used without its required template argument <T>, making the core AppEnum refactoring fundamentally broken.	High
Possible issue	Use canonical path for duplicate detection Comparing only file names will miss duplicates in different paths for non-JSON files and can also treat different files with the same name as duplicates. Use absolute canonical paths for all supported file types to robustly detect already-open files. This prevents duplicate loads and false positives. ApplicationLibCode/ProjectDataModel/RimWellPathCollection.cpp [254-274] for ( const QString& filePath : filePaths ) { - // Check if this file is already open + // Check if this file is already open by canonical path bool alreadyOpen = false; - if ( filePath.endsWith( "json", Qt::CaseInsensitive ) ) + QFile fNew( filePath ); + const QString newCanonical = QFileInfo( fNew ).canonicalFilePath(); + + for ( const auto& wellPath : m_wellPaths ) { - for ( const auto& wellPath : m_wellPaths ) + auto* fWPath = dynamic_cast<RimFileWellPath*>( wellPath.p() ); + if ( !fWPath ) continue; + + QFile fExisting( fWPath->filePath() ); + const QString existingCanonical = QFileInfo( fExisting ).canonicalFilePath(); + + if ( !newCanonical.isEmpty() && !existingCanonical.isEmpty() && newCanonical == existingCanonical ) { - auto* fWPath = dynamic_cast<RimFileWellPath*>( wellPath.p() ); - if ( !fWPath ) continue; - - QFile f1; - f1.setFileName( filePath ); - QString s1 = f1.fileName(); - QFile f2; - f2.setFileName( fWPath->filePath() ); - QString s2 = f2.fileName(); - if ( s1 == s2 ) - { - alreadyOpen = true; - errorMessages->push_back( QString( "%1 is already loaded" ).arg( filePath ) ); - break; - } + alreadyOpen = true; + errorMessages->push_back( QString( "%1 is already loaded" ).arg( filePath ) ); + break; } } + if ( !alreadyOpen ) + { + ... + } +} + [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that comparing only file names is a fragile way to detect duplicates and proposes a more robust solution using canonical file paths, which prevents both false positives and negatives.	Medium
Update