Skip to content

feat: Added key method of ListElement#257

Merged
simondsmart merged 3 commits intodevelopfrom
feature/pyfdb-listelement-key
Apr 7, 2026
Merged

feat: Added key method of ListElement#257
simondsmart merged 3 commits intodevelopfrom
feature/pyfdb-listelement-key

Conversation

@tbkr
Copy link
Copy Markdown
Contributor

@tbkr tbkr commented Mar 27, 2026

Description

For a ListElement we are now able to query the keys. The keys of the ListElement are returned as a dict[str, str] and resemble the MARS keys on the individual level of the Schema.

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

🌈🌦️📖🚧 Documentation FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/fdb/pull-requests/PR-257

@tbkr tbkr marked this pull request as ready for review March 27, 2026 14:59
@tbkr tbkr requested a review from Copilot March 27, 2026 15:00
@tbkr tbkr requested a review from danovaro March 27, 2026 15:04
Copy link
Copy Markdown

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 adds a way to query the (combined) MARS keys from a ListElement in the Python API, exposing them as a dict[str, str] via the pybind layer and the Python wrapper.

Changes:

  • Expose fdb5::ListElement::combinedKey() as a Python-accessible key() returning a dict[str, str] (via std::map).
  • Add a ListElement.key property (and expanded docstrings) in the Python wrapper.
  • Add an integration test exercising the new key attribute across list levels.

Reviewed changes

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

File Description
tests/pyfdb/integration/test_list.py Adds a parametrized integration test validating ListElement.key behavior per schema level.
src/pyfdb_bindings/bindings.cc Adds a ListElement.key binding that returns the combined key as a Python dict.
src/pyfdb/pyfdb_iterator.py Adds ListElement.key property + docstrings; also changes offset/length API shape/behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/pyfdb/pyfdb_iterator.py Outdated
Comment thread tests/pyfdb/integration/test_list.py Outdated
Comment thread src/pyfdb/pyfdb_iterator.py Outdated
Comment thread src/pyfdb/pyfdb_iterator.py Outdated
Comment thread src/pyfdb/pyfdb_iterator.py Outdated
@tbkr tbkr force-pushed the feature/pyfdb-listelement-key branch 4 times, most recently from 946741e to d334923 Compare March 27, 2026 15:11
@tbkr tbkr requested a review from simondsmart March 27, 2026 15:15
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.19%. Comparing base (ef36e0f) to head (def1c69).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #257      +/-   ##
===========================================
- Coverage    71.22%   71.19%   -0.03%     
===========================================
  Files          376      376              
  Lines        23762    23762              
  Branches      2478     2478              
===========================================
- Hits         16924    16918       -6     
- Misses        6838     6844       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tbkr tbkr force-pushed the feature/pyfdb-listelement-key branch from d334923 to 4c6a4ac Compare March 27, 2026 15:27
Copy link
Copy Markdown
Member

@danovaro danovaro left a comment

Choose a reason for hiding this comment

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

lgtm, only a small remark on the documentation

Comment thread src/pyfdb/pyfdb_iterator.py Outdated
For a ListElement we are now able to query the keys. The keys of the
ListElement are returned as a dict[str, str] and resemble the MARS keys
on the individual level of the Schema.
@tbkr tbkr force-pushed the feature/pyfdb-listelement-key branch from 4c6a4ac to 47ec20a Compare April 1, 2026 09:12
Copy link
Copy Markdown
Contributor

@simondsmart simondsmart left a comment

Choose a reason for hiding this comment

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

I would like to have a mechanism to extract either/both the 3 separated keys, correspnding to the the levels of the FDB, or the combined key. This only gives the combined key.

Copy link
Copy Markdown

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 8 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/pyfdb/integration/test_list.py Outdated
Comment thread tests/pyfdb/integration/test_list.py Outdated
Comment thread src/pyfdb/pyfdb_iterator.py Outdated
Comment thread src/pyfdb/pyfdb_iterator.py Outdated
Comment thread docs/pyfdb/examples.rst
Comment thread docs/pyfdb/examples.rst
Comment thread docs/pyfdb/examples.rst Outdated
Comment thread src/pyfdb/pyfdb_iterator.py
@tbkr tbkr force-pushed the feature/pyfdb-listelement-key branch from def1c69 to b841842 Compare April 1, 2026 16:56
@simondsmart simondsmart merged commit db7d78a into develop Apr 7, 2026
181 of 201 checks passed
@simondsmart simondsmart deleted the feature/pyfdb-listelement-key branch April 7, 2026 10:18
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