Skip to content

Issue #1810 fix weird gen warning#1833

Open
JoerivanEngelen wants to merge 6 commits intomasterfrom
issue_#1810_fix_weird_GEN_warning
Open

Issue #1810 fix weird gen warning#1833
JoerivanEngelen wants to merge 6 commits intomasterfrom
issue_#1810_fix_weird_GEN_warning

Conversation

@JoerivanEngelen
Copy link
Copy Markdown
Contributor

Fixes #1810

Description

This fixes confusing warning about incorrect IPF headers when loading GEN files.

  • Move _infer_delim_whitespace to a imod.formats.common.py module
  • Throw warning only in IPF module

Checklist

  • Links to correct issue
  • Update changelog, if changes affect users
  • PR title starts with Issue #nr, e.g. Issue #737
  • Unit tests were added
  • If feature added: Added/extended example
  • If feature added: Added feature to API documentation
  • If pixi.lock was changed: Ran pixi run generate-sbom and committed changes

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

@JoerivanEngelen JoerivanEngelen requested a review from Manangka May 7, 2026 13:31
Comment thread imod/formats/common.py
import csv


def infer_delimwhitespace(line: str, ncol: int):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This method does 2 things. From the name it isn't clear that it also does a column check.
You could make the name more generic and return a namedtuple.

The method signature than also contains the return type which makes it clearer what it does

class DelimiterInfo(NamedTuple):
    is_whitespace: bool
    has_expected_cols: bool


def infer_delimiter_info(line: str, ncol: int) -> DelimiterInfo:
    """
    Infer whether the line is delimited by whitespace or commas, and whether
    the number of comma-delimited columns matches the expected count.

    Parameters
    ----------
    line : str
        The line to analyze.
    ncol : int
        The expected number of columns if line is delimited by commas.

    Returns
    -------
    DelimiterInfo
        is_whitespace : bool
            Whether the line is delimited by whitespace.
        has_expected_cols : bool
            Whether the line has the expected number of columns if delimited by commas.
    """
    n_elem = len(next(csv.reader([line])))
    if n_elem == 1:
        return DelimiterInfo(is_whitespace=True, has_expected_cols=True)
    elif n_elem == ncol:
        return DelimiterInfo(is_whitespace=False, has_expected_cols=True)
    else:
        return DelimiterInfo(is_whitespace=False, has_expected_cols=False)

On the calling side you can then use the named tuple:

delimiter_info = infer_delimiter_info(line, ncol)`
delimiter_info.is_whitespace
delimiter_info.has_expected_cols

or directly unpack it:
has_whitespace, has_expected_cols = infer_delimiter_info(line, ncol)

Copy link
Copy Markdown
Collaborator

@Manangka Manangka May 7, 2026

Choose a reason for hiding this comment

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

infer_line_delimiter_info could also be a good name with the named tuple then being LineDelimiterInfo

from imod.formats.common import infer_delimwhitespace


def test_infer_delimwhitespace():
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This test assert many things.
Consider using test cases to make it clearer what you're testing

@pytest.mark.parametrize(
    "line, ncol, expected",
    [
        ("1 2 3", 3, (True, True)),    # whitespace-delimited, correct ncol
        ("1 2 3", 2, (True, True)),    # whitespace-delimited, wrong ncol: ncol is ignored
        ("1\t2\t3", 3, (True, True)),  # tab-delimited (csv.reader sees 1 element -> whitespace branch)
        ("1,2,3", 3, (False, True)),   # comma-delimited, correct cols
        ("1, 2, 3", 3, (False, True)), # comma-delimited with spaces, correct cols
        ("1,2,3", 4, (False, False)),  # comma-delimited, wrong number of cols
        ("1 2,3", 3, (False, False)),  # mixed delimiters, unexpected col count
    ],
)
def test_infer_delimiter_info(line, ncol, expected):
    assert infer_delimiter_info(line, ncol) == expected

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.

[Bug] - Confusing warning message when loading messy GEN files

2 participants