Add validation for FIELD parameter name

equinor / ert

ERT - Ensemble based Reservoir Tool - is designed for running ensembles of dynamical models such as reservoir models, in order to do sensitivity analysis and data assimilation. ERT supports data assimilation using the Ensemble Smoother (ES), Ensemble Smoother with Multiple Data Assimilation (ES-MDA) and Iterative Ensemble Smoother (IES).

https://ert.readthedocs.io/en/latest/

GNU General Public License v3.0

101 stars 104 forks source link

Add validation for FIELD parameter name #8248

Closed frode-aarstad closed 3 months ago

frode-aarstad commented 3 months ago

Issue Resolves #6070

Approach Short description of the approach

(Screenshot of new behavior in GUI if applicable)

[x] PR title captures the intent of the changes, and is fitting for release notes.
[x] Added appropriate release note label
[ ] Commit history is consistent and clean, in line with the contribution guidelines.
[ ] Make sure tests pass locally (after every commit!)

When applicable

[ ] When there are user facing changes: Updated documentation
[ ] New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
[ ] Large PR: Prepare changes in small commits for more convenient review
[ ] Bug fix: Add regression test for the bug
[ ] Bug fix: Create Backport PR to latest release

codecov-commenter commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.03%. Comparing base (98c9b5d) to head (8792a8d). Report is 1 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #8248 +/- ## ========================================== + Coverage 87.02% 87.03% +0.01% ========================================== Files 378 378 Lines 23952 23954 +2 Branches 634 620 -14 ========================================== + Hits 20844 20849 +5 + Misses 3030 3024 -6 - Partials 78 81 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dafeda commented 3 months ago

Do you know what the reasoning is behind this limit of 8 characters?

frode-aarstad commented 3 months ago

Do you know what the reasoning is behind this limit of 8 characters?

No, perhaps @berland knows..

berland commented 3 months ago

I suppose this parameter is meant to be exported as an Eclipse keyword. Eclipse limits keywords to 8 chars.

dafeda commented 3 months ago

I suppose this parameter is meant to be exported as an Eclipse keyword. Eclipse limits keywords to 8 chars.

Thanks, if that's the case we could add a small comment somewhere.

frode-aarstad commented 3 months ago

Suggest we add a comment where we raise error that the requirement is due to Eclipse.

I will add a comment to the exception and in the documentation