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.

Sign up for GitHub

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

Jump to bottom

Fix the definition of cartesian_site_positions length #216

Merged
merged 3 commits into from
Dec 4, 2019
Merged

Fix the definition of cartesian_site_positions length #216

merged 3 commits into from
Dec 4, 2019

Conversation

merkys
Copy link
Member

@merkys merkys commented Nov 29, 2019

Previously the length of cartesian_site_positions was defined as N times 3, where N is the number of sites in the structure. This is clearly incorrect, as the length of cartesian_site_positions MUST be equal to the number of sites. (cartesian_site_positions is list of lists, not a flat list).

@merkys merkys self-assigned this Nov 29, 2019
@merkys merkys added the type/bug A bug or problem in the specification. Needs to be addressed quickly. label Nov 29, 2019
@merkys merkys added this to the 1.0 release milestone Nov 29, 2019
rartino
rartino requested changes Dec 2, 2019
View reviewed changes
optimade.rst Outdated Show resolved Hide resolved
@merkys merkys requested a review from rartino December 2, 2019 08:39
CasperWA
CasperWA approved these changes Dec 2, 2019
View reviewed changes
Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @merkys, and for keeping in the original intent of the original line @rartino.

@CasperWA CasperWA added the PR/ready-for-review Add this flag if you are the author of the PR and you want it to be reviewed. Remove it when editing label Dec 2, 2019
@merkys merkys mentioned this pull request Dec 2, 2019
Open
@rartino rartino merged commit d13be6c into Materials-Consortia:develop Dec 4, 2019
@merkys merkys deleted the fix-cartesian_site_positions-dimension branch December 4, 2019 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rartino rartino rartino approved these changes

@CasperWA CasperWA CasperWA approved these changes

@sauliusg sauliusg Awaiting requested review from sauliusg

Assignees

@merkys merkys

Labels
PR/ready-for-review Add this flag if you are the author of the PR and you want it to be reviewed. Remove it when editing type/bug A bug or problem in the specification. Needs to be addressed quickly.
Projects
None yet
Milestone
1.0 release
Development

Successfully merging this pull request may close these issues.
3 participants
@merkys @rartino @CasperWA