Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix: Add arg detection for overloading #26

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

asoplata
Copy link
Contributor

This fixes #25 by customizing the calls to the overloaded function compute_distances_to_layer_boundaries. Since there is only one definition of compute_placement_hints, this should solve the problem in all cases. This has been tested against the exact command and data that @lecriste provided in #25 along with some other cases, and all tox tests have passed locally.

@asoplata asoplata changed the title Add arg detection for overloading Hotfix: Add arg detection for overloading Jul 10, 2024
lecriste
lecriste previously approved these changes Jul 10, 2024
Copy link

@lecriste lecriste left a comment

Choose a reason for hiding this comment

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

LGTM

@lecriste lecriste dismissed their stale review July 11, 2024 08:40

We should add a test for the isocortex option.

@arnaudon
Copy link
Contributor

Could you merge that? thanks!

@mgeplf
Copy link
Collaborator

mgeplf commented Jul 11, 2024

Could you merge that? thanks!

I haven't had a chance to make some changes - I'd prefer that None was used as the sentinel instead of an empty string, as an empty string is an allowable directory. I should be able to do it tomorrow.

@arnaudon
Copy link
Contributor

Ok, np!

@mgeplf
Copy link
Collaborator

mgeplf commented Jul 15, 2024

I haven't had time to refactor this, I will do it in another commit.
In the mean time, I will approve this, merge it and make a release.

@mgeplf mgeplf merged commit a99ba5e into BlueBrain:main Jul 15, 2024
4 checks passed
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.

Error in command atlas-placement-hints isocortex
4 participants