Add transition_counts() and transition_probabilities() methods - #47
Merged
Conversation
…hods Add methods to aggregate per-site transition data into square dict-of-dicts matrices. transition_counts() returns raw hop counts and transition_probabilities() returns row-normalised probabilities. Both methods accept a `by` parameter: 'site' (default) keys by site index; 'label' aggregates across sites sharing a label, skipping unlabelled sites. The output is always square with explicit zeros for missing transitions. Closes #45
- Use Literal['site', 'label'] for the `by` parameter on transition_counts() and transition_probabilities() for better static analysis and IDE support. - Guard against unknown destination indices in the by='site' path, emitting a warning and skipping rather than raising a KeyError. - Add test for self-transitions in by='site' mode. - Add test for zero-transition labels in by='label' probabilities.
- Add @overload signatures for transition_counts() and transition_probabilities() so callers get precise return types depending on the by='site' or by='label' argument. - Rename and expand docstring for self-transition test to clarify that self-transitions are not expected in normal trajectory data.
Replace dict-of-dicts and tuple returns with a frozen TransitionTable dataclass that provides multiple access patterns: - .matrix for the raw numpy array - .loc[from_key, to_key] for key-based lookup - .to_dict() for square dict-of-dicts transition_counts() and transition_probabilities() now return TransitionTable directly, with an optional keys parameter for custom row/column ordering. This removes the need for separate _matrix methods, @overload signatures, and union return types.
- Rename data field to matrix; drop redundant .matrix property - Make keys a tuple and matrix read-only for true immutability - Add __eq__ (using np.array_equal) and __hash__ - Move reorder() onto TransitionTable; require exact key permutation - Unknown destination site index now raises ValueError (was warning) - Transitions to/from unlabelled sites now emit a warning with count - Add NaN post-condition assertion in transition_probabilities() - Tighten type hints on _LocAccessor and to_dict() - Use relative import for transition_table - Export TransitionTable from __init__.py - Add tests for read-only matrix, reorder validation, equality, custom keys with by='site' probabilities, and subset key rejection
- Replace assert with RuntimeError for NaN check (not stripped by -O) - Defensive copy of input matrix in TransitionTable.__post_init__ - Add type: ignore[arg-type] on intentionally invalid by='invalid' tests
The dataclass was generating only __init__ while we overrode __eq__, __hash__, __repr__, and fought frozen=True with object.__setattr__ calls. A plain class with __slots__ and a __setattr__ guard is more honest about the immutability contract and simpler to read.
- Use explicit _frozen sentinel for immutability guard instead of order-dependent hasattr check. - Catch duplicate keys in reorder() with a length check before the set comparison, giving a clearer error message. - Drop redundant tuple[()] from key type union. - Include offending keys and row sums in NaN RuntimeError message.
Remove _LocAccessor helper class and cached _loc field. Key-based lookup is now table.get(from_key, to_key) with @overload signatures enforcing matched key types (int, int or str, str).
There was a problem hiding this comment.
Pull request overview
Adds a structured, immutable TransitionTable abstraction and exposes new Trajectory APIs to aggregate per-site transition Counter data into square count/probability matrices for downstream analysis.
Changes:
- Introduce
site_analysis.transition_table.TransitionTable(immutable keys + matrix,.get(),.to_dict(),.reorder()). - Add
Trajectory.transition_counts()andTrajectory.transition_probabilities()returningTransitionTableforby='site'andby='label'aggregation modes (with optional key ordering and label-mode warnings). - Add unit tests covering
TransitionTablebehaviors and the new trajectory transition APIs; exportTransitionTablefromsite_analysis.__init__.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_transition_table.py | New unit tests for TransitionTable construction, immutability, accessors, reorder, equality, repr. |
| tests/test_trajectory.py | New unit tests for Trajectory.transition_counts() / transition_probabilities() across by modes and custom key ordering. |
| site_analysis/transition_table.py | New TransitionTable implementation providing immutable, labelled square matrix utilities. |
| site_analysis/trajectory.py | Adds transition aggregation/normalisation methods producing TransitionTable. |
| site_analysis/init.py | Re-exports TransitionTable for external consumption. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- In by='label', validate destination indices against all site indices and raise ValueError for truly unknown indices (not just unlabelled). - Count transitions from unlabelled source sites in the dropped total. - Remove redundant np.array() copy in reorder(). - Update docstring to reflect .get() API (was .loc[]).
There was a problem hiding this comment.
Pull request overview
This PR adds a new TransitionTable value object and exposes new Trajectory.transition_counts() / Trajectory.transition_probabilities() APIs to return transition data as a labelled, reorderable square matrix (counts or row-normalised probabilities).
Changes:
- Added
site_analysis.transition_table.TransitionTable(immutable, key-indexed square matrix with.get(),.to_dict(),.reorder()). - Added
Trajectory.transition_counts()andTrajectory.transition_probabilities()producingTransitionTableforby='site'andby='label'(with optional key ordering and warnings for dropped unlabelled transitions). - Added unit tests covering
TransitionTablebehavior and trajectory transition aggregation/normalisation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
site_analysis/transition_table.py |
Introduces TransitionTable immutable wrapper around a square NumPy matrix with key-based access and reordering. |
site_analysis/trajectory.py |
Implements transition_counts() and transition_probabilities() returning TransitionTable for site/label aggregation. |
site_analysis/__init__.py |
Exports TransitionTable at package level for downstream imports/type checking. |
tests/test_transition_table.py |
Adds construction/immutability/access/reorder/equality/repr tests for TransitionTable. |
tests/test_trajectory.py |
Adds tests for transition counts/probabilities (site/label), custom key ordering, and validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- In by='label', validate destination indices against all site indices even when the source site is unlabelled, before skipping. - Add divide='ignore' to np.errstate alongside invalid='ignore' to suppress the divide-by-zero warning from np.where on zero-sum rows.
Only divide rows with nonzero sums instead of dividing all rows and masking with np.where. Removes the need for np.errstate and the NaN post-condition check.
There was a problem hiding this comment.
Pull request overview
Adds a new TransitionTable value object and exposes new Trajectory APIs to compute transition count/probability tables for downstream analysis, including site- and label-aggregated views.
Changes:
- Introduces
site_analysis.transition_table.TransitionTable(immutable, keyed square matrix with.get(),.to_dict(), and.reorder()). - Adds
Trajectory.transition_counts()andTrajectory.transition_probabilities()returningTransitionTable, supportingby='site'|'label'and optional custom key ordering. - Adds/extends unit tests for
TransitionTableand the newTrajectorytransition APIs; exportsTransitionTablefromsite_analysis.__init__.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
site_analysis/transition_table.py |
New TransitionTable implementation used as the return type for transition aggregation APIs. |
site_analysis/trajectory.py |
Implements transition_counts() and transition_probabilities() with site/label aggregation and key reordering. |
site_analysis/__init__.py |
Exposes TransitionTable at package top-level for downstream imports/type checking. |
tests/test_transition_table.py |
New unit tests validating TransitionTable construction, accessors, immutability, reorder, and equality semantics. |
tests/test_trajectory.py |
New unit tests covering transition counts/probabilities across by modes, warnings, and key reordering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TransitionTable has no use case as a dict key or set member, and the mutable numpy array makes hash invariants fragile. Replace __hash__ with explicit None to make the type unhashable.
There was a problem hiding this comment.
Pull request overview
Adds a first-class TransitionTable container and new Trajectory helpers to expose transition counts/probabilities in a consistent, matrix-backed format (supporting site- and label-aggregation) for downstream analysis.
Changes:
- Introduce immutable
TransitionTable(keyed square matrix +.get(),.to_dict(),.reorder()). - Add
Trajectory.transition_counts()andTrajectory.transition_probabilities()returningTransitionTable, withby='site'|'label'and optional custom key ordering. - Add comprehensive unit tests covering construction/validation, aggregation logic, normalization behavior, warnings, and key reordering.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
site_analysis/transition_table.py |
New TransitionTable implementation (validation, accessors, reorder, immutability). |
site_analysis/trajectory.py |
Adds transition_counts() / transition_probabilities() producing TransitionTable for site/label aggregation. |
site_analysis/__init__.py |
Exports TransitionTable from package root for downstream imports/type checking. |
tests/test_transition_table.py |
New unit tests for TransitionTable behaviors and validation. |
tests/test_trajectory.py |
New unit tests for trajectory transition counts/probabilities (site/label modes, ordering, validation). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ensures deterministic error messages regardless of set ordering.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TransitionTableimmutable class for encapsulating transition data with multiple access patterns:.matrixfor the raw (read-only) numpy array.get(from_key, to_key)for key-based lookup.to_dict()for square dict-of-dicts conversion.reorder(keys)for reordering rows/columns (must be a permutation)Trajectory.transition_counts(by='site'|'label')aggregates per-site transition data into aTransitionTableof integer countsTrajectory.transition_probabilities(by='site'|'label')row-normalises counts into aTransitionTableof probabilitieskeysparameter for custom row/column ordering (default is sorted)by='label'aggregates across sites sharing a label; unlabelled sites are skipped (with a warning if transitions are dropped)ValueErrorTransitionTableexported fromsite_analysis.__init__for downstream type checkingCloses #45