Skip to content

Vector bugfix#48

Merged
aligfellow merged 10 commits intoaligfellow:mainfrom
scohenjanes5:vector-bugfix
Mar 13, 2026
Merged

Vector bugfix#48
aligfellow merged 10 commits intoaligfellow:mainfrom
scohenjanes5:vector-bugfix

Conversation

@scohenjanes5
Copy link
Contributor

This fixes a bug for still images of pbc systems with both axes and custom vectors requested where neither or only one was shown.

I made additional changes to treat these two vector sources the same by combining the sources of vectors early, so a consistent set of transformations can be applied without duplicating work.

I also unified the scattered means by which rotation is applied so it is the same scheme for vectors, the lattice box and all tests.

@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 71.11111% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/xyzrender/api.py 71.42% 5 Missing and 3 partials ⚠️
src/xyzrender/utils.py 33.33% 4 Missing ⚠️
src/xyzrender/viewer.py 83.33% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@aligfellow
Copy link
Owner

Thanks for your work on this!

I made a couple of changes. Running something like this scaled all vectors, including the crystallographic axes. I added an "is_axis" flag to the VectorArrow, which lets us control the vector annotation scale separate from the axes.

xyzrender examples/structures/caffeine_cell.xyz --cell --vectors examples/strucutres/ethanol_dip.json --vector-scale 10 

Stylistically, the --vectors flag was plural, and other vector_scale was singluar. I changed the vector flag to be singular (sorry if thats annoying), but I've updated the docs/readme to fit that. This now fits stylistically with label.

@aligfellow aligfellow merged commit 0ac85fd into aligfellow:main Mar 13, 2026
1 check 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