Refactor API for better consistency and documentation #4

MitchellAcoustics · 2025-02-26T13:36:36Z

Summary

Standardize naming conventions (ssm_analyze instead of ssm_analyse)
Add comprehensive docstrings using Google style
Fix circular imports between modules
Add centralized parameter validation
Improve plotting interface with consistent methods
Fix deprecation warnings in pandas groupby operations

Changes

This PR refactors the API to make it more consistent and better documented, while maintaining backward compatibility. Key improvements include:

API Consistency

Standardized function names across the codebase
Added better parameter validation with informative error messages
Fixed circular imports between modules using TYPE_CHECKING

Documentation

Added comprehensive docstrings to all public functions
Added code examples for key functions
Improved parameter descriptions

Improved Interface

Added convenience plotting methods to SSMResults
Made error handling more consistent (ValueError instead of assertions)
Fixed deprecation warnings in pandas operations

Testing

Updated tests to work with the new validation approach
All tests pass with the refactored API

🤖 Generated with Claude Code

We are beginning to build out all of the same functionality of the R package.

The confidence intervals and plotting don't work correctly when the bounds are forced to (0, 2pi). Therefore we allow negative angles for the CIs only.

- Standardize naming with ssm_analyze instead of ssm_analyse - Add comprehensive docstrings using NumPy style - Add examples to key functions - Fix circular imports between modules - Add centralized parameter validation - Improve typing with more specific return annotations - Add consistent plot methods to SSMResults

…ersions

…analyse functions

Copilot

Pull Request Overview

This PR represents a major refactoring of the circumplex package, transitioning from a monolithic design to a modular architecture. The changes split the code into separate modules for analysis, visualization, utilities, and data structures, while adding comprehensive test coverage and new instrument definitions.

Modularized codebase by splitting circumplex.py into ssm_analysis.py, visualization.py, utils.py, and ssm_results.py
Added comprehensive test suite with visualization, analysis, and notebook validation tests
Introduced new instrument definitions (IIPSC) and enhanced existing ones with normative data support

Reviewed Changes

Copilot reviewed 25 out of 31 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
tests/test_visualisation.py	New test suite for visualization functions including circular plots and profile plots
tests/test_ssm_analysis.py	New comprehensive tests for SSM analysis with parametrized test cases
tests/test_notebooks.py	New notebook structure validation tests
tests/test_analysis.py	Removed legacy test file, functionality moved to test_ssm_analysis.py
src/circumplex/visualization.py	New module containing all plotting functions (ssm_plot, ssm_plot_circle, ssm_profile_plot)
src/circumplex/utils.py	New utility module with mathematical functions (cosine_form, r2_score, angle_median)
src/circumplex/ssm_results.py	New SSMResults class with plotting and data access methods
src/circumplex/ssm_analysis.py	New analysis module with ssm_analyze and related functions
src/circumplex/instrument.py	Enhanced instrument handling with normative data support and improved API
src/circumplex/instruments/*.json	Updated instrument definitions with consistent field naming (inst_items)
src/circumplex/core/	New core module directory (appears to be experimental/unused)
pyproject.toml	Updated dependencies and build system from pdm to hatchling
requirements*.lock	New lock files for dependency management
docs/tutorials/using-instruments.ipynb	New tutorial notebook for instrument usage

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

Copilot · 2025-11-08T12:15:34Z

src/circumplex/core/__init__.py

@@ -0,0 +1,9 @@
+from instrument import Instrument, instruments


The import statement uses a relative import from instrument import ... without a leading dot, which will fail. This should be either from .instrument import ... for a relative import or from circumplex.core.instrument import ... for an absolute import.

Suggested change

from instrument import Instrument, instruments

from .instrument import Instrument, instruments

Copilot · 2025-11-08T12:15:34Z

src/circumplex/visualization.py

+    if incl_amp:
+        ax.axhline(amplitude + elevation, color="black", linestyle="--")
+        ax.text(
+                0, elevation + amplitude * 0.5, f"a = {amplitude:.2f}", fontsize=fontsize


Incorrect calculation for amplitude text position. The text placement uses elevation + amplitude * 0.5, but this doesn't properly center the text at the midpoint of the amplitude line. It should be elevation + amplitude / 2 or elevation + amplitude * 0.5 needs to be calculated from the baseline, not the elevation.

Suggested change

0, elevation + amplitude * 0.5, f"a = {amplitude:.2f}", fontsize=fontsize

0, elevation + amplitude / 2, f"a = {amplitude:.2f}", fontsize=fontsize

Copilot · 2025-11-08T12:15:34Z

tests/test_visualisation.py

+
+
+def test_ssm_plot_contrast(mock_ssm_results):
+    fig = visualization.ssm_plot_circle(mock_ssm_results)


The test function name test_ssm_plot_contrast doesn't match its implementation. It calls ssm_plot_circle instead of ssm_plot_contrast, making it a duplicate of test_ssm_plot_circle rather than testing contrast plots.

Suggested change

fig = visualization.ssm_plot_circle(mock_ssm_results)

fig = visualization.ssm_plot_contrast(mock_ssm_results)

Copilot · 2025-11-08T12:15:35Z

src/circumplex/visualization.py

+                fc=list(colors[i]) + [0.5],  # face opacity at 0.5
+            linestyle=row["linestyle"],
+                edgecolor=list(colors[i]) + [1],  # edge opacity at 1


The list concatenation with + [0.5] and + [1] may fail or behave unexpectedly. colors[i] returns a tuple/array from seaborn palette, so list(colors[i]) creates a list that gets concatenated with a single-element list. Consider using tuple unpacking or (*colors[i], 0.5) for cleaner code, or ensure the result is a tuple: fc=tuple(list(colors[i]) + [0.5]).

Suggested change

fc=list(colors[i]) + [0.5], # face opacity at 0.5

linestyle=row["linestyle"],

edgecolor=list(colors[i]) + [1], # edge opacity at 1

fc=(*colors[i], 0.5), # face opacity at 0.5

linestyle=row["linestyle"],

edgecolor=(*colors[i], 1), # edge opacity at 1

Copilot · 2025-11-08T12:15:35Z

src/circumplex/ssm_analysis.py

+if __name__ == "__main__":
+    ######## SCRATCH ########
+    from importlib.resources import files
+
+    import matplotlib.pyplot as plt
+
+    _jz2017_path = str(files("circumplex.data").joinpath("jz2017.csv"))
+    data = pd.read_csv(_jz2017_path)
+
+    results = ssm_analyze(
+        data=data,
+        scales=["PA", "BC", "DE", "FG", "HI", "JK", "LM", "NO"],
+            angles=(90, 135, 180, 225, 270, 315, 0, 45),
+        # grouping="Gender",
+        # contrast='model',
+        measures=["NARPD", "ASPD"],
+        # measures_labels=['Narcissistic PD', 'Antisocial PD'],
+    )
+    print(results.scores)
+    # print(results)
+    print(results.summary())
+
+    # fig = ssm_plot(results)
+    fig, ax = results.profile_plot(incl_elev=True)
+    plt.show()


There is executable scratch/debug code at the module level (lines 846-870) that should not be in production code. This code will execute whenever the module is imported by other modules. Move this code to a separate script or protect it with if __name__ == "__main__": guard that's already present at line 846.

Copilot · 2025-11-08T12:15:35Z

src/circumplex/core/__init__.py

+
+__all__ = [
+    "ssm_analyze",
+    "SSMResults",


The name 'SSMResults' is exported by all but is not defined.

Copilot · 2025-11-08T12:15:36Z

src/circumplex/core/__init__.py

+__all__ = [
+    "ssm_analyze",
+    "SSMResults",
+    "ssm_plot",


The name 'ssm_plot' is exported by all but is not defined.

Copilot · 2025-11-08T12:15:36Z

src/circumplex/ssm_results.py

@@ -0,0 +1,154 @@
+from __future__ import annotations
+
+from typing import Any, Dict, List, Tuple, Optional


Import of 'Optional' is not used.

Suggested change

from typing import Any, Dict, List, Tuple, Optional

from typing import Any, Dict, List, Tuple

Copilot · 2025-11-08T12:15:36Z

src/circumplex/utils.py

@@ -0,0 +1,127 @@
+from typing import List, Tuple, Union, Optional, Any


Import of 'Optional' is not used.
Import of 'Any' is not used.

Suggested change

from typing import List, Tuple, Union, Optional, Any

from typing import List, Tuple, Union

Copilot · 2025-11-08T12:15:36Z

src/circumplex/visualization.py

@@ -0,0 +1,489 @@
+from __future__ import annotations
+
+from typing import List, Optional, Tuple, Union, Any


Import of 'Any' is not used.

Suggested change

from typing import List, Optional, Tuple, Union, Any

from typing import List, Optional, Tuple, Union

MitchellAcoustics and others added 30 commits March 23, 2024 10:13

Added instruments() function to list all instruments

50c0298

We are beginning to build out all of the same functionality of the R package.

Added items to instrument.Instrument() class

2a6c65f

Change to rye packaging

84bda4b

Add IIPSC instrument

aaf41a0

Started building out Intro_to_SSM_Analysis.ipynb tutorial

483be5a

Add using.instruments.ipynb tutorial to mkdocs.yml

6876fd4

fix bug in readthedocs.yml

bfba7bc

still fixing bug in readthedocs.yml

9c2be56

still fixing bug in readthedocs.yml

4975d12

Adding to Intro_to_SSM_Analysis.ipynb tutorial

a17d4f3

Added show methods to instrument components

0f30aeb

customize number of items printed

10d9c3c

Added iipsc data

c6370ea

Direct implementation of R ssm_analysis()

848fe54

Implement circle plotting

1ce506d

changed plotting to seaborn

bf18bb6

fix bug with test palette

733c5f1

formatting

a06b70d

refactored into four main files

2913794

implement profile plots

6de9f69

fix bug in profile plot

77ba165

add option for elevation hline

88d852d

rename profile plot function

35d69cb

restructure whole module

eafb967

ssm_parameters always accepts and returns degrees

2b4438b

Add visualisation tests

3565192

Revert to allowing negative displacements for cis

8217bb3

The confidence intervals and plotting don't work correctly when the bounds are forced to (0, 2pi). Therefore we allow negative angles for the CIs only.

Update Python version, project dependencies and add uv.lock

04e05bb

Fix type annotation in utils.py to avoid circular import issues

e570708

MitchellAcoustics added 6 commits February 26, 2025 13:28

Update tests to accept ValueError for invalid inputs

5481b65

Fix pandas DeprecationWarning in groupby operation

465fa7c

Remove include_groups parameter for compatibility with older pandas v…

705c577

…ersions

Update Random_exs.ipynb to use ssm_analyze instead of deprecated ssm_…

71b4691

…analyse functions

Add test for notebook structure validation

b073819

test notebooks

dc71a26

MitchellAcoustics requested a review from Copilot November 8, 2025 12:11

Copilot AI reviewed Nov 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor API for better consistency and documentation #4

Refactor API for better consistency and documentation #4

Uh oh!

MitchellAcoustics commented Feb 26, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Copilot AI Nov 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -0,0 +1,9 @@
		from instrument import Instrument, instruments

	from instrument import Instrument, instruments
	from .instrument import Instrument, instruments

	0, elevation + amplitude * 0.5, f"a = {amplitude:.2f}", fontsize=fontsize
	0, elevation + amplitude / 2, f"a = {amplitude:.2f}", fontsize=fontsize



		def test_ssm_plot_contrast(mock_ssm_results):
		fig = visualization.ssm_plot_circle(mock_ssm_results)

	fig = visualization.ssm_plot_circle(mock_ssm_results)
	fig = visualization.ssm_plot_contrast(mock_ssm_results)

		@@ -0,0 +1,154 @@
		from __future__ import annotations

		from typing import Any, Dict, List, Tuple, Optional

		@@ -0,0 +1,127 @@
		from typing import List, Tuple, Union, Optional, Any

	from typing import List, Tuple, Union, Optional, Any
	from typing import List, Tuple, Union

		@@ -0,0 +1,489 @@
		from __future__ import annotations

		from typing import List, Optional, Tuple, Union, Any

Refactor API for better consistency and documentation #4

Are you sure you want to change the base?

Refactor API for better consistency and documentation #4

Uh oh!

Conversation

MitchellAcoustics commented Feb 26, 2025

Summary

Changes

API Consistency

Documentation

Improved Interface

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant