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

bug fix: string truncation in AtomArray annotations #755

Merged

Conversation

davegrays
Copy link
Contributor

@davegrays davegrays commented Feb 14, 2025

The structure.array() method truncates any string-based annotations to an arbitrary length of string. i.e. you can end up with

atoms[1000] --> Atom(..., str_annot="111", ...)
atoms[1000].str_annot.dtype --> dtype('<U3')

array = structure.array(atoms)
array[1000] --> Atom(..., str_annot="1", ...)
array[1000].str_annot.dtype --> dtype('<U1')

The culprit is this
array.add_annotation(name, dtype=type(atoms[0]._annot[name]))
is using the first atom in the array, which is often the shortest, to determine the dtype of the whole array. This is particularly problematic when processing cif columns that contains string representations of ids. Instead, we need to find the maximum length across the atom list to ensure no truncations occur.

@davegrays davegrays force-pushed the fix/atomarray-string-annotations branch from c67b22b to 1effc13 Compare February 14, 2025 04:56
Copy link

codspeed-hq bot commented Feb 14, 2025

CodSpeed Performance Report

Merging #755 will not alter performance

Comparing davegrays:fix/atomarray-string-annotations (1effc13) with main (d7df095)

Summary

✅ 59 untouched benchmarks

Copy link
Member

@padix-key padix-key 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 the fix 👍!

@padix-key
Copy link
Member

The currently failing tests are caused be a change on the side on the RCSB, which will be addressed in #759. Hence, I will merge this PR.

@padix-key padix-key merged commit 89f9510 into biotite-dev:main Feb 15, 2025
27 of 28 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.

2 participants