Skip to content

Conversation

@larsevj
Copy link
Collaborator

@larsevj larsevj commented Oct 9, 2025

No description provided.

@larsevj larsevj force-pushed the add_wildcard_of_wlist_compdat branch from 39a409b to 5253858 Compare October 9, 2025 12:12
@larsevj larsevj requested a review from Copilot October 9, 2025 12:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements wildcard expansion for well names in WLIST when processing COMPDAT keywords. The changes enable WLIST entries containing wildcard patterns (like "*OP") to be expanded to actual well names found in the COMPDAT data at the corresponding date.

  • Adds wildcard expansion functionality to WLIST processing by matching patterns against wells defined in COMPDAT
  • Refactors WLIST expansion into separate functions for wildcard expansion and action expansion
  • Updates function signatures to pass COMPDAT data to WLIST expansion functions

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/res2df/compdat.py Implements wildcard expansion logic and refactors WLIST expansion functions
tests/test_wlist.py Updates test to use the new function name for WLIST action expansion
tests/test_welopen.py Adds test case for wildcard expansion in WELOPEN with WLIST

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@larsevj larsevj requested a review from Copilot October 13, 2025 14:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/res2df/compdat.py:1

  • Multiple spelling errors: 'wilcard' should be 'wildcard', 'charachters' should be 'characters' (appears twice).
"""Parser and dataframe generator for the keywords:

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@larsevj larsevj force-pushed the add_wildcard_of_wlist_compdat branch from 258a13e to 3702907 Compare October 14, 2025 12:34
@larsevj larsevj force-pushed the add_wildcard_of_wlist_compdat branch from 3702907 to 53e01c7 Compare October 16, 2025 12:39
@larsevj larsevj changed the title Try add wildcard expansion of wlist wells in compdat Add wildcard expansion of wlist wells in compdat Oct 16, 2025
@larsevj
Copy link
Collaborator Author

larsevj commented Oct 16, 2025

In test_wlist.py there is the following test case:

        # Wildcard wells should pass through, the dataframe user
        # will have to process it.
        (
            pd.DataFrame(
                [
                    {
                        "NAME": "OP",
                        "ACTION": "NEW",
                        "WELLS": "PROD*",
                        "DATE": datetime.date(1900, 1, 1),
                    },
                ]
            ),
            pd.DataFrame(
                [
                    {
                        "NAME": "OP",
                        "ACTION": "NEW",
                        "WELLS": "PROD*",
                        "DATE": datetime.date(1900, 1, 1),
                    },
                ]
            ),
        ),
    ],

It is unclear to me if this was due to the functionality not being in place at the time, for instance via the get_wells_matching_template function added in #333 or if there are any specific reasons or pitfalls related to supporting this functionality.
Do you remember anything @berland ?

@berland
Copy link
Collaborator

berland commented Oct 16, 2025

89d14d7 indicates that there might be a reason not to process wildcards in WLIST parsing.

@larsevj
Copy link
Collaborator Author

larsevj commented Oct 16, 2025

89d14d7 indicates that there might be a reason not to process wildcards in WLIST parsing.

Do you know what the commit message Can be handled in WELOPEN instead., refers to? As I can currently understand, you end up inserting from wlist_df into welopen_df, and afterwards you try and match up the wells in welopen_df and compdat_df, but since they can be wildcarded in wlist_df -> welopen_df you run the risk of them not matching up with compdat_df as seen in https://equinor.slack.com/archives/CMS2PTC3Z/p1759499665007219

But there is an expansion of wildcards in welopen_df, only it happens before wells from wlist_df is inserted into welopen_df. So maybe if you do the wildcards expansion of welopen afterwards it would work, but then I do not see why you would not do it directly in wlist_df. Probably something I am missing here.

@berland
Copy link
Collaborator

berland commented Oct 20, 2025

There is a comment in the commit mentioned above:

        # Wildcard wells are currently not supported
        # (it requires knowledge of all currenly processed wells)

I don't remember details here, but if you are parsing the entire deck when doing this, and thus maintain the state of which wells are currently defined, it could work.

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.

3 participants