Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add ecl2df as utility #198

Merged
merged 33 commits into from
Aug 31, 2023
Merged

Add ecl2df as utility #198

merged 33 commits into from
Aug 31, 2023

Conversation

daniel-sol
Copy link
Contributor

Solving #197. But suggesting that utility should be called sim2sumo.
*Add metadata to export of csv files with library ecl2df

  • uploads to sumo
  • Controls export and upload with yaml config file

upload_with_config(args.config_path, args.env)


# @hook_implementation
Copy link
Member

Choose a reason for hiding this comment

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

Intentionally left in the code?

Copy link
Member

@perolavsvendsen perolavsvendsen left a comment

Choose a reason for hiding this comment

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

Some small comments here and there, but some more fundamental questions:

  • Has this been tested with e.g. Drogon or other functional FMU workflows using a komodo environment (e.g. do we know that the job installs and works?)
  • As commented in the code, not all fields use the conventional output for global_variables. Is it possible to provide a custom path?

@perolavsvendsen perolavsvendsen requested a review from a team August 28, 2023 14:08
src/fmu/sumo/utilities/scripts/sim2sumo.py Outdated Show resolved Hide resolved
tests/test_utility_sim2sumo.py Outdated Show resolved Hide resolved
tests/test_utility_sim2sumo.py Outdated Show resolved Hide resolved
@daniel-sol daniel-sol merged commit e348f5b into main Aug 31, 2023
2 checks passed
@daniel-sol daniel-sol deleted the add_ecl2df_as_utility branch August 31, 2023 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants