Skip to content

fix: correct Synapse base class, stale docstrings, and minor type issues#792

Open
haoyu-haoyu wants to merge 2 commits into
jaxleyverse:mainfrom
haoyu-haoyu:fix/minor-bugs-and-docs
Open

fix: correct Synapse base class, stale docstrings, and minor type issues#792
haoyu-haoyu wants to merge 2 commits into
jaxleyverse:mainfrom
haoyu-haoyu:fix/minor-bugs-and-docs

Conversation

@haoyu-haoyu
Copy link
Copy Markdown

Summary

  • Add missing self parameter to Synapse.update_states() and Synapse.compute_current() in jaxley/synapses/synapse.py. All concrete subclasses (IonotropicSynapse, TanhConductanceSynapse, etc.) correctly include self, but the base class methods were missing it.

  • Update current unit in docstrings from uA/cm2 to mA/cm2 in jaxley/channels/channel.py. The unit was changed in v0.5.0 (with a runtime check enforcing current_is_in_mA_per_cm2 = True), but the class docstring and compute_current return docstring still referenced the old unit.

  • Fix invalid type annotation np.ndarray[bool]np.ndarray in jaxley/connect.py (relates to issue faulty type subscript #615). np.ndarray does not support subscript type parameters in standard Python.

  • Remove duplicate Callable import in jaxley/integrate.py. It was imported from both collections.abc (Python 3.9+ preferred) and typing.

Test plan

  • No behavioral changes — only signature corrections, docstring fixes, and type annotation cleanup
  • All concrete Synapse subclasses already have self and are unaffected
  • Existing tests should pass without modification

… issues

- Add missing `self` parameter to `Synapse.update_states()` and
  `Synapse.compute_current()` base class methods. All concrete
  subclasses already include `self`; the base class was inconsistent.

- Update current unit in docstrings from `uA/cm2` to `mA/cm2` in
  `channel.py`. The unit was changed in v0.5.0 but the docstrings
  were not updated.

- Fix invalid `np.ndarray[bool]` type annotation in `connect.py`.
  `np.ndarray` does not support subscript type parameters.

- Remove duplicate `Callable` import in `integrate.py`. It was
  imported from both `collections.abc` and `typing`.
Copy link
Copy Markdown
Contributor

@michaeldeistler michaeldeistler left a comment

Choose a reason for hiding this comment

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

Thanks for opening this! All proposed changes look good to me.

Could you add a line to the CHANGELOG.md which briefly describes these changes? Thank you!

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.

2 participants