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

add lipidorderkit to mdanalysis #149

Merged
merged 13 commits into from
Jul 17, 2024
Merged

Conversation

ricard1997
Copy link
Contributor

This code allows for the computation of lipid order parameters for all-atom MDAsimulation for researchers that study membrane simulations.

Copy link
Member

@lilyminium lilyminium left a comment

Choose a reason for hiding this comment

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

Hi @ricard1997, thanks for adding your MDAKit to the registry -- exciting to see more lipid analyses here! I've just done a quick first pass over the metadata file and commented on where some entries need some modification to be properly parsed by the MDAKits registry.

mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
mdakits/lipidorderkit/metadata.yaml Outdated Show resolved Hide resolved
@IAlibay
Copy link
Member

IAlibay commented Jul 7, 2024

CI is failing due to a typo - I opened a PR in the mdakit to fix this.

@ricard1997
Copy link
Contributor Author

CI is failing due to a typo - I opened a PR in the mdakit to fix this.

I have accepted the PR. Let me know if something else is needed.

@lilyminium
Copy link
Member

Thanks for updating the file @ricard1997! All looking pretty good -- however I think CI is failing due to not finding test files, which I hopefully fixed in ricard1997/lipidorderkit#3.

@ricard1997
Copy link
Contributor Author

Thanks for updating the file @ricard1997! All looking pretty good -- however I think CI is failing due to not finding test files, which I hopefully fixed in ricard1997/lipidorderkit#3.

Thanks for the help with the problem with pytest!
I have solved the problem with the CI, and think everything is finally ready. Thanks for the feedback!

@ricard1997 ricard1997 requested a review from IAlibay July 15, 2024 21:29
Copy link
Member

@lilyminium lilyminium left a comment

Choose a reason for hiding this comment

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

All LGTM -- thank you for making the changes @ricard1997!! Very happy to see a new lipid kit in the registry.

@lilyminium lilyminium merged commit 5a39e05 into MDAnalysis:main Jul 17, 2024
5 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.

3 participants