Skip to content

Fix turbine_type search parameter in get_oedb_windturbineconfig - #519

Open
dylanpulver wants to merge 1 commit into
PyPSA:masterfrom
dylanpulver:fix-oedb-turbine-type-search
Open

Fix turbine_type search parameter in get_oedb_windturbineconfig#519
dylanpulver wants to merge 1 commit into
PyPSA:masterfrom
dylanpulver:fix-oedb-turbine-type-search

Conversation

@dylanpulver

Copy link
Copy Markdown

Changes proposed in this Pull Request

get_oedb_windturbineconfig documents turbine_type as a recognized search
argument, but the branch that applies it reads the wrong key:

if "turbine_type" in search_params:
    selector &= df.turbine_type.str.contains(search_params["name"], case=False)

Two consequences on current master, both reproduced against the live OEDB
wind_turbine_library endpoint the function queries:

  • get_oedb_windturbineconfig(turbine_type="E-101/3050") raises
    KeyError: 'name'.
  • get_oedb_windturbineconfig(name="E-101/3050", turbine_type="__nope__")
    returns E-101/3050. The turbine type is silently ignored rather than
    narrowing the search.

The two columns are not interchangeable. Of the 68 library entries that carry
a power curve, 58 have name != turbine_type, 51 have a turbine_type that
is not a substring of its own name, and 7 have no name at all (several
Nordex rows), so for those the turbine type is the only string that
identifies them. manufacturer and name are unaffected and are used as
controls in the new tests.

The fix passes search_params["turbine_type"]. Introduced in 12846ce
(v0.2 preparation, Jan 2021).

The three new tests patch the module-level _oedb_turbines cache with a
two-row frame, so they are deterministic and make no network call. Reverting
the fix fails the first two; filtering the right value against the name
column fails the first; deleting the branch fails both. test_resource.py
previously asserted only that a result was truthy and never passed
turbine_type=, which is why the defect survived.

Test suite: 83 passed / 38 skipped on master → 86 / 38 with this branch,
the +3 being exactly the new tests. The 38 skips are the CDS-credentialed
cutout tests and are skipped identically either way, so that path is unverified
here. ruff check and ruff format --check are
clean across the repo.

Written with AI assistance; the behaviour above was measured, not inferred.

Checklist

  • Code changes are sufficiently documented; i.e. new functions contain docstrings and further explanations may be given in doc.
  • Unit tests for new features were added (if applicable).
  • Newly introduced dependencies are added to environment.yaml, environment_docs.yaml and setup.py (if applicable).
  • A note for the release notes doc/release_notes.rst of the upcoming release is included.
  • I consent to the release of this PR's code under the MIT license.

The `turbine_type` branch filtered the `turbine_type` column with the
value of `search_params["name"]`. Searching by `turbine_type` alone
raised `KeyError: 'name'`, and passing both silently ignored the
requested turbine type.

In the OEDB wind turbine library the two columns differ for 58 of the
68 entries that carry a power curve, so the two are not
interchangeable.
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.

1 participant