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

Matplotlib CSM visualization should display units for axis labels #80

Open
marscher opened this issue Jun 2, 2022 · 5 comments
Open
Labels
enhancement New feature or request

Comments

@marscher
Copy link
Contributor

marscher commented Jun 2, 2022

Assuming all LCS have the same units (is this case treated in CSM?) we should draw the unit for each axis label.

@marscher marscher added the enhancement New feature or request label Jun 2, 2022
@vhirtham
Copy link

vhirtham commented Jun 3, 2022

If we want to label the axes, we can simply iterate over all LCS, retrieve the coordinate unit and take the most common one for the plot. In this case, it wouldn't matter if all have the same or not. Additionally, we could add a unit parameter to the plot function so that the user can select his favorite unit. ;)

@marscher
Copy link
Contributor Author

marscher commented Jun 3, 2022

But shouldn't the least common unit be converted inside the LCS to the most common one? Otherwise there is a great potential for confusion.
Agreed upon adding a unit parameter (which would then also trigger a convert).

@vhirtham
Copy link

vhirtham commented Jun 3, 2022

Not sure why. As long as you "work" with the CSM all unit-related stuff is handled internally the moment it is relevant. If you extract an LCS, coordinates have still units attached. Only if you want to visualize it, a common unit is needed. But we can solve this the way I suggested.

@CagtayFabry
Copy link
Member

we could also try enabling the matplotlib support of pint for our unit registry, ideally that would not require any conversion on our part but make it easy to set the desired output unit

@marscher
Copy link
Contributor Author

marscher commented Aug 2, 2022

This sounds like a very convenient solution. Thanks for pointing it out.

@marscher marscher transferred this issue from BAMWelDX/weldx-widgets Aug 2, 2022
@marscher marscher transferred this issue from BAMWelDX/weldx Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants