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 of no space in the result of to_string. #14379
Conversation
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Telomelonia - thanks for this! Some small nitpicky comments only. With those, this will be all set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the changes this pull request is proposing the following works as it should:
>>> from astropy.coordinates import Angle
>>> Angle(5, unit="rad").to_string()
'5 rad'
But the following still produce a string without a space (it would be good to add a unit test analogous to one of these):
>>> Angle(5, unit="rad").to_string(format="unicode")
'5rad'
>>> Angle(5, unit="rad").to_string(format="generic")
'5rad'
It would seem that a space should not be added between the value and the unit if the format is "latex"
or "latex_inline"
, or if the output is sexagesimal.
Oops, indeed, this is more of a partial fix than I had realized! I think in principle there should be a space also for One option here may be to just keep it simple and focus on the generic format only -- i.e., only update this PR to also allow |
I think one way is to include every format possible where space is required, since few units don't require space like |
My sense is still that apart from sexagesimal, this should all be handled by My sense, though, is not to let that stand in the way of doing the smaller change here, which solves things mostly. |
198c064
to
a865798
Compare
@Telomelonia - I squashed your commits and added an extra one that also adds a space for other formats. I think this way it is ready to go in! @eerovaher - would you mind having another look? I think your comment is now addressed. |
a865798
to
51bdf2d
Compare
Hopefully now got the |
Are the "TODO"s in original post above still relevant? |
@pllim - I think they're basically done with my additional commit, so I ticked them. |
51bdf2d
to
4c12086
Compare
Requested changes have been implemented
Thanks, all! |
Thank you all for assisting me |
Description
As mentioned in the issue, in the result of the to_string() function, there is no space between value and unit.So, I added a space between value and unit.
This pull request is to address ...
Fixes #7455
Close #14231
TODOS