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

[BUG] settings dictionary not parsed correctly for WellModel #508

Closed
dbrakenhoff opened this issue Jan 30, 2023 · 0 comments
Closed

[BUG] settings dictionary not parsed correctly for WellModel #508

dbrakenhoff opened this issue Jan 30, 2023 · 0 comments
Assignees
Labels
bug Indicates an unintended behavior or coding error

Comments

@dbrakenhoff
Copy link
Member

dbrakenhoff commented Jan 30, 2023

Describe the bug
When pastas loads time series settings contained in a StressModel, it checks whether the length of the list of settings is >1, and if that is not the case it squeezes the output by selecting the only entry in that list. This causes issues in WellModel when there are multiple stresses but the settings is a single dictionary. There was logic for extending the settings as a list of dict with length 1 if it did not match the number of stresses, but not for a single dictionary.

Code to reproduce
See XFAIL test in pastastore:
https://github.com/pastas/pastastore/blob/d5ab8e4f07c4d134621848fe8b9c2b4a76f6bdb3/tests/test_004_yaml.py#L128-L141

@pytest.mark.dependency()
@pytest.mark.xfail(reason="settings not parsed correctly in pastas")
def test_write_yaml_minimal_nearest(request, pstore):
    depends(
        request,
        [
            f"test_load_yaml_rechargemodel[{pstore.type}]",
            f"test_load_yaml_stressmodel[{pstore.type}]",
            f"test_load_yaml_wellmodel[{pstore.type}]",
        ],
    )
    ml = pstore.models["my_third_model"]
    pstore.yaml.export_model(ml, minimal_yaml=True, use_nearest=True)

Expected behavior
Deal with settings as a dictionary, automatically extend settings to apply for all time series. Easy fix by also checking if settings is a dictionary and applying it to all stresses. In WellModel.__init__ ():

        if settings is None or isinstance(settings, str) or isinstance(settings, dict):  # <- add isinstance check for dict here
            settings = len(stress) * [settings]

Python package version
pastas 0.23.0

Additional context
I'm not sure if this bug every bothers pastas scripts directly but it occurs with the pastastore YAML interface. Related to changes for #411 .

@dbrakenhoff dbrakenhoff added the bug Indicates an unintended behavior or coding error label Jan 30, 2023
@raoulcollenteur raoulcollenteur added this to the 1.0: Arrabiata milestone Jan 31, 2023
dbrakenhoff added a commit that referenced this issue Feb 1, 2023
parse settings as single dict for wellmodel
This was referenced Feb 1, 2023
@dbrakenhoff dbrakenhoff mentioned this issue Feb 1, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unintended behavior or coding error
Projects
None yet
Development

No branches or pull requests

2 participants