Namespace ies #2621

joakim-hove · 2021-12-20T09:58:46Z

Introduce namespace ies::

Have opted to ies:: explicitly in the implementation, instead of enclosing implementation files in namespace ies { ... } - that is to avoid "visual name confusion".

codecov-commenter · 2021-12-20T10:18:11Z

Codecov Report

Merging #2621 (0dd98aa) into main (4fb977e) will increase coverage by 4.09%.
The diff coverage is 68.93%.

@@            Coverage Diff             @@
##             main    #2621      +/-   ##
==========================================
+ Coverage   60.95%   65.05%   +4.09%     
==========================================
  Files         339      651     +312     
  Lines       36582    53908   +17326     
  Branches     4540     4610      +70     
==========================================
+ Hits        22300    35069   +12769     
- Misses      12827    17358    +4531     
- Partials     1455     1481      +26

Impacted Files	Coverage Δ
libres/lib/analysis/analysis_module.cpp	`45.81% <0.00%> (ø)`
libres/lib/analysis/ies/ies.cpp	`56.36% <53.98%> (ø)`
libres/lib/analysis/ies/ies_data.cpp	`83.70% <82.05%> (ø)`
libres/lib/analysis/ies/ies_config.cpp	`90.00% <92.30%> (ø)`
libres/lib/analysis/std_enkf.cpp	`63.69% <92.85%> (ø)`
libres/lib/res_util/block_fs.cpp	`53.23% <0.00%> (ø)`
libres/lib/include/ert/python.hpp
ert_gui/plottery/plot_style.py	`97.75% <0.00%> (ø)`
ert3/console/_status.py	`100.00% <0.00%> (ø)`
ert_gui/model/snapshot.py	`86.23% <0.00%> (ø)`
... and 332 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fb977e...0dd98aa. Read the comment docs.

joakim-hove · 2022-01-04T16:30:11Z

Ready for review from my side

ManInFez · 2022-01-05T15:04:14Z

libres/lib/analysis/ies/ies_enkf.cpp

-    ies_enkf_config_type *config = ies_enkf_data_get_config(module_data);
+namespace {
+
+bool set_int(void *arg, const char *var_name, int value) {


Might be a stupid question, but why are these not in the ies namespace

They are private - accessed through the functions pointers in struct analysis_table_type.

oyvindeide

Thanks for the PR, looks like a good improvement! Some small comments, but the nature of the subject means most of them are opinionated. I think it could befit for a further splitting up though, in the way of a nested namespace, as the remaining function names often point to that I feel.

libres/lib/analysis/ies/ies_enkf.cpp

libres/lib/analysis/ies/ies_enkf_data.cpp

libres/lib/include/ert/analysis/ies/ies_enkf_config.hpp

joakim-hove · 2022-01-07T06:48:55Z

Hmmm - these failing equinor tests I do not have control over.

joakim-hove · 2022-01-07T11:01:47Z

Can this be merged?

joakim-hove · 2022-01-10T06:34:26Z

ModuleNotFoundError: No module named 'starlette.graphql'

dafeda · 2022-01-10T07:55:17Z

ModuleNotFoundError: No module named 'starlette.graphql'

Seems like all PR's are getting this.
We're looking into it.

libres/lib/include/ert/analysis/ies/ies_config.hpp

libres/lib/include/ert/analysis/std_enkf.hpp

pinkwah

🥳

joakim-hove · 2022-01-11T08:10:35Z

test ert please

joakim-hove · 2022-01-11T14:22:20Z

How can I force a rerun of the Equinor tests?

dafeda · 2022-01-11T15:29:36Z

test ert please

joakim-hove force-pushed the namespace-ies branch 2 times, most recently from 71b0366 to 1cd1c8b Compare January 4, 2022 16:22

joakim-hove force-pushed the namespace-ies branch 2 times, most recently from 08665bc to 697602a Compare January 5, 2022 09:20

ManInFez reviewed Jan 5, 2022

View reviewed changes

oyvindeide reviewed Jan 6, 2022

View reviewed changes

libres/lib/analysis/ies/ies_enkf.cpp Outdated Show resolved Hide resolved

libres/lib/analysis/ies/ies_enkf_data.cpp Outdated Show resolved Hide resolved

libres/lib/include/ert/analysis/ies/ies_enkf_config.hpp Outdated Show resolved Hide resolved

joakim-hove force-pushed the namespace-ies branch 3 times, most recently from c6988c8 to 0eba55c Compare January 6, 2022 14:36

joakim-hove closed this Jan 6, 2022

joakim-hove reopened this Jan 6, 2022

joakim-hove closed this Jan 6, 2022

joakim-hove reopened this Jan 6, 2022

joakim-hove closed this Jan 7, 2022

joakim-hove reopened this Jan 7, 2022

joakim-hove closed this Jan 7, 2022

joakim-hove reopened this Jan 7, 2022

joakim-hove force-pushed the namespace-ies branch from 0eba55c to d17168f Compare January 10, 2022 06:25

pinkwah reviewed Jan 10, 2022

View reviewed changes

libres/lib/include/ert/analysis/ies/ies_config.hpp Outdated Show resolved Hide resolved

libres/lib/include/ert/analysis/std_enkf.hpp Show resolved Hide resolved

pinkwah approved these changes Jan 10, 2022

View reviewed changes

Introduce namespace ies

97ebacb

joakim-hove force-pushed the namespace-ies branch from 2de7561 to 97b7b99 Compare January 10, 2022 10:15

joakim-hove mentioned this pull request Jan 10, 2022

Use ies::config::config class for configuration also of std enkf module #2681

Merged

joakim-hove closed this Jan 11, 2022

joakim-hove reopened this Jan 11, 2022

joakim-hove closed this Jan 11, 2022

joakim-hove reopened this Jan 11, 2022

joakim-hove enabled auto-merge (rebase) January 12, 2022 07:30

joakim-hove mentioned this pull request Jan 12, 2022

Namespace ies2 #2694

Closed

joakim-hove added 3 commits January 12, 2022 08:38

Update comment to get a new sha

204ad50

Remove prefix enkf_ from all functions in the ies namespace

b0f2717

Add sub namespace ies::config::

20c8bbe

joakim-hove force-pushed the namespace-ies branch from 97b7b99 to 8bc0590 Compare January 12, 2022 07:42

joakim-hove disabled auto-merge January 12, 2022 08:07

joakim-hove added 2 commits January 12, 2022 11:23

Remove prefix config_ from ies::config functions

5934a6c

Remove unnecessary C includes

0dd98aa

joakim-hove force-pushed the namespace-ies branch from 8bc0590 to 0dd98aa Compare January 12, 2022 10:24

joakim-hove merged commit 1d3e102 into equinor:main Jan 12, 2022

joakim-hove deleted the namespace-ies branch January 12, 2022 10:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Namespace ies #2621

Namespace ies #2621

joakim-hove commented Dec 20, 2021 •

edited

codecov-commenter commented Dec 20, 2021 •

edited

joakim-hove commented Jan 4, 2022

ManInFez Jan 5, 2022

joakim-hove Jan 5, 2022

ManInFez Jan 6, 2022

oyvindeide left a comment

joakim-hove commented Jan 7, 2022

joakim-hove commented Jan 7, 2022

joakim-hove commented Jan 10, 2022

dafeda commented Jan 10, 2022

pinkwah left a comment

joakim-hove commented Jan 11, 2022

joakim-hove commented Jan 11, 2022

dafeda commented Jan 11, 2022

Namespace ies #2621

Namespace ies #2621

Conversation

joakim-hove commented Dec 20, 2021 • edited

codecov-commenter commented Dec 20, 2021 • edited

Codecov Report

joakim-hove commented Jan 4, 2022

ManInFez Jan 5, 2022

Choose a reason for hiding this comment

joakim-hove Jan 5, 2022

Choose a reason for hiding this comment

ManInFez Jan 6, 2022

Choose a reason for hiding this comment

oyvindeide left a comment

Choose a reason for hiding this comment

joakim-hove commented Jan 7, 2022

joakim-hove commented Jan 7, 2022

joakim-hove commented Jan 10, 2022

dafeda commented Jan 10, 2022

pinkwah left a comment

Choose a reason for hiding this comment

joakim-hove commented Jan 11, 2022

joakim-hove commented Jan 11, 2022

dafeda commented Jan 11, 2022

joakim-hove commented Dec 20, 2021 •

edited

codecov-commenter commented Dec 20, 2021 •

edited