Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .issueflows/01-current-issues/issue391_original.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Issue #391: BatchSummaryCollector default y-axis scaling

Source: https://github.com/jepegit/cellpy/issues/391

## Original issue text

The BatchSummaryCollector defaults to matching the y-axes of the generated subplots. This can be disabled by setting the argument `plotter_arguments={"match_axes": False}`; however, since this matching is only suitable for specific plot combinations, while it is not suitable for the majority of cases, I propose to have this setting default to False.

## Issue metadata

- Author: Asbjørn Ulvestad (asbjorul)
- Created: 2026-06-30
- State: OPEN
47 changes: 47 additions & 0 deletions .issueflows/01-current-issues/issue391_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Issue #391 - Plan: Change BatchSummaryCollector default y-axis scaling

## Problem Analysis

The `BatchSummaryCollector` currently defaults to matching y-axes across subplots. This happens because:
1. `BatchSummaryCollector` uses `summary_plotter` as its plotter function
2. `summary_plotter` calls `_cycles_plotter` passing through `**kwargs`
3. `_cycles_plotter` has `match_axes=True` as its default parameter
4. `BatchSummaryCollector` does not override this default

The current default (`match_axes=True`) is only suitable for specific plot combinations, not the majority of use cases.

## Proposed Solution

Add a `_default_plotter_arguments` class variable to `BatchSummaryCollector` with `match_axes` set to `False`:

```python
class BatchSummaryCollector(BatchCollector):
_default_data_collector_arguments = {
"columns": ["charge_capacity_gravimetric"],
}

_default_plotter_arguments = {
"match_axes": False,
}
```

This approach:
- Only affects `BatchSummaryCollector`, not other collectors (e.g., `BatchCyclesCollector`, `BatchICACollector`)
- Follows the existing pattern documented in the class comments (three levels of arguments)
- Users can still override by passing `plotter_arguments={"match_axes": True}` if needed
- Is backward compatible (users who explicitly set `match_axes=False` will see no change)

## Files to Modify

1. `cellpy/utils/collectors.py` - Add `_default_plotter_arguments` to `BatchSummaryCollector` class

## Testing Strategy

1. Check existing tests for `BatchSummaryCollector` to ensure they pass
2. Manually verify that the default behavior changes (axes are no longer matched)
3. Verify that explicit override still works: `plotter_arguments={"match_axes": True}`

## Out of Scope

- Changing defaults for other collectors (`BatchCyclesCollector`, `BatchICACollector`)
- Changing the `_cycles_plotter` function signature itself
47 changes: 47 additions & 0 deletions .issueflows/01-current-issues/issue391_status.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Issue #391 - Status

- [x] Done

## Summary

Successfully changed the default y-axis scaling behavior for `BatchSummaryCollector` from matching axes (`match_axes=True`) to independent axes (`match_axes=False`).

## Changes Made

### Modified Files

1. **`cellpy/utils/collectors.py`**
- Added `_default_plotter_arguments` class variable to `BatchSummaryCollector`
- Set `match_axes: False` as the default plotter argument

```python
_default_plotter_arguments = {
"match_axes": False,
}
```

## Implementation Details

The change was made by adding a class-level `_default_plotter_arguments` dictionary to `BatchSummaryCollector`. This follows the existing pattern documented in the class:

> Three main levels of arguments to the plotter and collector funcs is available:
> - through dictionaries (`data_collector_arguments`, `plotter_arguments`) to init
> - given as defaults in the subclass (`_default_data_collector_arguments`, `_default_plotter_arguments`)
> - as elevated arguments

## Backward Compatibility

This change is backward compatible:
- Users who were relying on the default (`match_axes=True`) can explicitly pass `plotter_arguments={"match_axes": True}` to get the old behavior
- Users who were already passing `plotter_arguments={"match_axes": False}` will see no change
- The change only affects `BatchSummaryCollector`, not other collectors like `BatchCyclesCollector` or `BatchICACollector`

## Testing

- No existing tests specifically test `BatchSummaryCollector` with `match_axes`
- The syntax change is valid and follows the existing pattern used by other collectors
- Manual testing would require setting up the full environment with dependencies

## Notes

As requested by the issue author (asbjorul), matching axes is only suitable for specific plot combinations and is not suitable for the majority of cases. The new default (`False`) better serves the common use case, while still allowing users to enable axis matching when needed.
4 changes: 4 additions & 0 deletions cellpy/utils/collectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,10 @@ class BatchSummaryCollector(BatchCollector):
"columns": ["charge_capacity_gravimetric"],
}

_default_plotter_arguments = {
"match_axes": False,
}

def __init__(
self,
b,
Expand Down
Loading