Skip to content

Add unit test for dics_coherence_external#14

Merged
wmvanvliet merged 3 commits into
AaltoImagingLanguage:mainfrom
ruuskas:external
Oct 8, 2025
Merged

Add unit test for dics_coherence_external#14
wmvanvliet merged 3 commits into
AaltoImagingLanguage:mainfrom
ruuskas:external

Conversation

@ruuskas

@ruuskas ruuskas commented Oct 7, 2025

Copy link
Copy Markdown
Contributor

Basically as title says. It's not perfect, but 100% coverage except for the case of several external signals.

I was unable to run the entire test suite, but at least this test works on my machine.

Comment thread tests/test_connectivity.py Outdated
dics_coherence_external
)
from conpy.connectivity import _BaseConnectivity, _get_vert_ind_from_label
from conpy.connectivity import BaseConnectivity, _get_vert_ind_from_label

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I changed this as some point. _BaseConnectivity should be a private class.

Comment thread tests/test_connectivity.py Outdated
@@ -382,7 +403,7 @@ def test_compatibility():
# Test BaseConnectivity
assert not baseCon.is_compatible(label_con)

@wmvanvliet wmvanvliet Oct 8, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert not baseCon.is_compatible(label_con)
assert not con_base.is_compatible(label_con)

The python convention is to use snake case 🐍 for variable names (and camel case for class names).

@wmvanvliet

Copy link
Copy Markdown
Collaborator

I've added CI robots that run the entire test suite. They are currently unhappy because of the BaseConnectivity / _BaseConnectivity issue.

@ruuskas

ruuskas commented Oct 8, 2025

Copy link
Copy Markdown
Contributor Author

The robots seem to be happy now

@wmvanvliet

Copy link
Copy Markdown
Collaborator

very nice work, thanks!

@wmvanvliet
wmvanvliet merged commit d8913e1 into AaltoImagingLanguage:main Oct 8, 2025
2 checks passed
@ruuskas
ruuskas deleted the external branch October 9, 2025 07:43
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