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

Use points instead of diameters for MorphIO conversion #47

Merged
merged 3 commits into from
Mar 7, 2022

Conversation

adrien-berchet
Copy link
Member

#45 was merged in the wrong branch, this PR just cherry-picks the commit to merge it on the master branch.

@eleftherioszisis
Copy link
Collaborator

May I ask why it is preferable to copy a 2D dataset instead of a 1D to get its length?

@adrien-berchet
Copy link
Member Author

May I ask why it is preferable to copy a 2D dataset instead of a 1D to get its length?

Which copy? The points or diameters are not copied, we just use them to know the total number of points.

@arnaudon
Copy link
Collaborator

lol: ERROR: Could not build wheels for morphio, which is required to install pyproject.toml-based projects

@eleftherioszisis
Copy link
Collaborator

You will need to remove python3.6 as morphio does not create wheels for it after support for python3.6 was dropped.

@eleftherioszisis
Copy link
Collaborator

May I ask why it is preferable to copy a 2D dataset instead of a 1D to get its length?

Which copy? The points or diameters are not copied, we just use them to know the total number of points.

Data copied from the c++ to the python side. The only reason I used diameters is that they are 1D so would result in a smaller copy. That said, it doesn't really matter because the datasets are rather small.

@adrien-berchet
Copy link
Member Author

You will need to remove python3.6 as morphio does not create wheels for it after support for python3.6 was dropped.

Ah ok, thanks.

@adrien-berchet
Copy link
Member Author

May I ask why it is preferable to copy a 2D dataset instead of a 1D to get its length?

Which copy? The points or diameters are not copied, we just use them to know the total number of points.

Data copied from the c++ to the python side. The only reason I used diameters is that they are 1D so would result in a smaller copy. That said, it doesn't really matter because the datasets are rather small.

Yeah I think we can use the points for now, and when BlueBrain/MorphIO#357 will be solved we will use it.

@adrien-berchet
Copy link
Member Author

@eleftherioszisis do you have the permissions to discard the py36 required job?

@adrien-berchet
Copy link
Member Author

Hello there,
Do you know if a new release is planned for MorphIO? BlueBrain/MorphIO#357 was merge but we can't use it here without a new release.

@adrien-berchet
Copy link
Member Author

@eleftherioszisis should we merge this one and use n_points method later or should we wait for a new release of MorphIO?

@eleftherioszisis
Copy link
Collaborator

May I ask why it is preferable to copy a 2D dataset instead of a 1D to get its length?

Which copy? The points or diameters are not copied, we just use them to know the total number of points.

Data copied from the c++ to the python side. The only reason I used diameters is that they are 1D so would result in a smaller copy. That said, it doesn't really matter because the datasets are rather small.

Yeah I think we can use the points for now, and when BlueBrain/MorphIO#357 will be solved we will use it.

Sorry for responding late. n_points should be available now.

tmd/io/conversion.py Outdated Show resolved Hide resolved
tmd/io/conversion.py Outdated Show resolved Hide resolved
tmd/io/conversion.py Outdated Show resolved Hide resolved
tmd/io/conversion.py Outdated Show resolved Hide resolved
@adrien-berchet adrien-berchet merged commit 8a6f7bb into master Mar 7, 2022
@adrien-berchet adrien-berchet deleted the no_diameters branch March 7, 2022 12:34
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.

None yet

3 participants