Repository navigation
feat(utils): Add timeseries conversion utilities for sktime - #2225
SarthakB11 wants to merge 6 commits into
Conversation
|
@ankurankan please review. |
|
This PR has had no activity for 5 days and is now marked as stale. |
|
This PR has had no activity for 5 days and is now marked as stale. |
b27616c to
7970640
Compare
|
Hi @ankurankan, reviving this PR with significant updates:
All CI green: pre-commit, CodeQL, docs, doctests (3.10 + 3.14), notebook tests, full test suite (core + optional deps), codecov/patch, codecov/project. The implementation is self-contained in |
|
@SarthakB11 Can you update the PR checklist? The template is here: https://github.com/pgmpy/pgmpy/blob/dev/.github/PULL_REQUEST_TEMPLATE.md Please also review our new AI usage policy as outlined in the Contribution guide. |
@ankurankan Done! I've updated it as per the new template. |
|
Gentle ping @ankurankan — checklist updated and CI is green; happy to address any further comments. |
Remove redundant `drop(columns="__instance__")` call that fails on pandas 3.x since `set_index` already consumes the column. Add 8 edge-case tests covering all previously uncovered branches: error handling (bad MultiIndex, missing instance_col, non-tuple cols), single-trajectory fallback, unbalanced panel NaN filling, and round-trip without time reverse map.
- Replace typing.List/Optional with PEP 604 style (list[...], str | None) - Move Hashable import to collections.abc - Apply ruff-format line wrapping
0a72668 to
9875817
Compare
|
This PR has had no activity for 21 days and is now marked as stale. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #2225 +/- ##
==========================================
+ Coverage 95.72% 95.74% +0.01%
==========================================
Files 585 587 +2
Lines 33074 33215 +141
==========================================
+ Hits 31661 31801 +140
- Misses 1413 1414 +1
|
Your checklist for this pull request
Have you followed all the steps from our Contributing Guide?
Does the PR fully address the linked issue and is within its defined scope? If you are still working on the PR, mark it as draft.
Are all the GitHub Actions checks passing? If not, mark your PR as draft while you fix it.
Have you reviewed our AI usage policy?
Are you able to fully explain your changes? We expect you to fully understand the algorithm and take full responsibility for any changes in this PR.
Please describe in detail how and what you used AI assistance for? Please outline your whole workflow with the AI tool.
For revival, I used Claude and spawned clawteam agents for planning and approved it at each step. I guided it until the plan was fine and then started implementing. I rebased onto upstream/dev and resolved merge conflict. The prior 203 line implementation of from_sktime_to_dbn / from_dbn_to_sktime, _canonical_time_order, _build_time_mapping in pgmpy/utils/timeseries.py, 2 tests (test_multiindex_roundtrip, test_instance_col_roundtrip) in pgmpy/tests/test_timeseries_utils.py and updates to pgmpy/utils/init.py were done using Chatgpt for planning and Cursor for implementation.
I ran tests locally and found pandas 3.x bug. Test had failed with KeyError : "['instance'] not found in axis" from line 105's drop(columns="instance"). Claude applied the fix and wrote 8 new edge case tests (code + docstrings). When I ran those tests, the tests failed with an error. Claude identitifed the root cause and fixed one test assertion (result.loc[row, tuple_col] → result[tuple_col].loc[row] as pandas interprets the tuple as two separate column labels). Then I ran ruff autofix and force pushed with --force-with-lease. I used AI for the maintainer ping comment posted 2026-04-11.
All the issue asks (from_sktime_to_dbn(df, instance_col=None), from_dbn_to_sktime(dbn_data), Tests in tests/test_timeseries_utils.py, DateTime -> int mapping, Overlap with existing pgmpy utilities) have been addressed in the PR.
The edge cases that I have covered in the tests are 3-level MultiIndex -> ValueError to reject malformed input, 2-level MultiIndex with arbitrary time labels, Flat DataFrame + instance_col argument, Flat DataFrame, no instance_col -> single-trajectory fallback to treat all rows as instance 0, instance_col that doesn't exist in columns -> KeyError, Unbalanced panels that have one instance has fewer time steps than another -> missing cells filled with NaN and column grid stays rectangular, DBN -> sktime with non-tuple columns -> ValueError, DBN -> sktime with 3-tuple columns -> ValueError, DBN -> sktime with _time_reverse_map missing -> still works, returns integer time labels.
I have verified round trip fidelity via tests, coverage, CI matrix and the module adds new files only so there is no change to existing code paths.
Yes, I have used Claude for writing the tests. The tests are one-per-branch. Collapsing them would lose the line-mapping.
Issue number(s) that this pull request fixes
List of changes to the codebase in this pull request
pgmpy/utils/timeseries.pywith functions to convert between sktime panel DataFrames and pgmpy DBN wide DataFrames.from_sktime_to_dbnandfrom_dbn_to_sktimefor robust, round-trip conversions.pgmpy/tests/test_timeseries_utils.py.pgmpy/utils/__init__.pyto export the new utilities.Results