Skip to content

Conversation

@AurelienJaquier
Copy link
Contributor

This has to be merged once this entitysdk PR has been merged: openbraininstitute/entitysdk#139

@AurelienJaquier
Copy link
Contributor Author

AurelienJaquier commented Oct 28, 2025

pinging @darshanmandge on all my unmerged PRs
This is blocked bu the entitysdk PR.

Copy link
Collaborator

@chr-pok chr-pok left a comment

Choose a reason for hiding this comment

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

The code looks fine to me!

Just wondering, is there an example notebook so I can actually run this code? If not it would be great to add a minimal working example.

Copy link
Collaborator

@james-isbister james-isbister left a comment

Choose a reason for hiding this comment

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

There is a notebook examples/A-.../ion_channel_modeling.ipynb @chr-pok

Currently failing when the useIon model is built within the task

grid_scan.execute()

with pytest.raises(ValueError, match=re.escape("Unknown properties: [np.str_('INVALID')]")):
with pytest.raises(ValueError, match=r"Unknown properties:.*INVALID"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I changed this is in main recently to:
[np.str_('INVALID')]

Do we know why this change was needed? It looks like connectome utilities hasn't been changed @chr-pok

Copy link
Collaborator

Choose a reason for hiding this comment

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

No idea! I cannot reproduce locally. I suppose some dependencies that are used by connectome utilities have changed perhaps.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, then this is a good fix for now I think!

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.

5 participants