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

Squared Column/Quantity does not square unit (v3.1.2) #8918

Closed
mireianievas opened this issue Jun 25, 2019 · 14 comments
Closed

Squared Column/Quantity does not square unit (v3.1.2) #8918

mireianievas opened this issue Jun 25, 2019 · 14 comments
Labels
Docs Effort-low good first issue Issues that are well-suited for new contributors Package-novice table units

Comments

@mireianievas
Copy link

mireianievas commented Jun 25, 2019

I am pretty surprised to see this behavior with some astropy quantities in 3.1.2 while working with some gamma-ray data.

image

Is this expected?

@mireianievas mireianievas changed the title Squared Quantity does not square unit (v3.1.2) Squared Column/Quantity does not square unit (v3.1.2) Jun 25, 2019
@mireianievas
Copy link
Author

Funnily enough, doing energies.to(energies.unit) right before doing all the operations makes it behave as I would expect.

@pllim
Copy link
Member

pllim commented Jun 25, 2019

@mireianievas , thanks for reporting this. Just so we are all on the same page, can you please provide a minimal example of how the input table was constructed? Also, please provide Numpy version.

@mireianievas
Copy link
Author

mireianievas commented Jun 27, 2019

#!/bin/env python3
import astropy
from astropy.table.column import Column
import astropy.units as units
energies = Column(data=[1,2,3,4],name="E",unit="TeV")
# returns the column with TeV as unit:
print(energies)
# returns the column with contents^2 but units unchanged:
print(energies**2)
# returns the column with contents^2 but units unchanged:
print(energies*energies)
# .to() converts the column into a Quantity, then the units are transformed as expected:
print(energies.to("TeV")**2)

@pllim
Copy link
Member

pllim commented Jun 27, 2019

@taldcroft or @mhvk , can you please comment on the reported Column behavior?

@mireianievas , FWIW if you pass the column into a QTable, it seems to work. For example:

>>> from astropy.table import QTable
>>> t = QTable([energies])
>>> t['E'] ** 2
<Quantity [ 1.,  4.,  9., 16.] TeV2>

@mhvk
Copy link
Contributor

mhvk commented Jun 27, 2019

@mireianievas - Column instances treat the unit as just part of the description, not something that has meaning in operations. In your case (and generally) this is not what you want, but we cannot easily change it (both for backwards compatibility reasons and because Column supports masking, while Quantity does not). In your case, definitely follow @pllim's advise and use QTable. For more details, see http://docs.astropy.org/en/latest/table/mixin_columns.html#quantity-and-qtable

@mhvk
Copy link
Contributor

mhvk commented Jun 27, 2019

p.s. Looking at the documentation, it took me a bit more time than I had hoped to find the above link.
@taldcroft - might it make sense to discuss QTable right from the get-go, maybe even have the examples with it?

@pllim pllim added Docs Package-novice Effort-low good first issue Issues that are well-suited for new contributors labels Jun 27, 2019
@taldcroft
Copy link
Member

Definitely agree. I would suggest to remove the early example that begins with "A column with a unit works with and can be easily converted to an Quantity object (but see Quantity and QTable for a way to natively use Quantity objects in tables):".

Replace that with an example that says, if you are working with data that have units, use QTable and Quantity. End of story. Then deep in the docs mention that Column has some quantity-like behavior that provides limited compatibility but is generally not recommended.

@taldcroft
Copy link
Member

And 👍 to generally scrubbing the table docs to give more emphasis to QTable. E.g. it might be fine and appropriate to put the QTable example as the 2nd example in the Table Getting Started.

@mhvk
Copy link
Contributor

mhvk commented Jun 27, 2019

Sounds good! @mireianievas - in many ways, changes to the documentation are best done by people who just learned how things work, rather than by developers who often implicitly assume things. Any chance you might be able to help make the changes? (Definitely do not feel obligated; just having your issue here clearly already helped, as it made us realize the documentation could be better.)

@mireianievas
Copy link
Author

Hi, thanks for all the info. I looked into the wiki and to be honest, it is pretty clear as it is now, now it makes total sense. I just had the doubt because I was not expecting that behavior and it is not straightforward to come with the right keywords to look for it. But probably I should have spent a bit more time looking for this.

@pllim
Copy link
Member

pllim commented Jun 28, 2019

@mireianievas , no worries. If you had problem finding the info, you won't be the only one. The documentation could be improved. Thanks for looking into this, @taldcroft and @mhvk !

@opg7371
Copy link

opg7371 commented Mar 20, 2020

Hey...I would like to address this issue. Is it okay @mhvk?

@taldcroft
Copy link
Member

This was addressed to some extent by #9143, in particular starting off right away with using QTable. However @opg7371 if you want to put in a few more lines here or there then feel free!

@mhvk
Copy link
Contributor

mhvk commented Apr 12, 2021

Closing this since #9143 addressed this mostly.

@mhvk mhvk closed this as completed Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Effort-low good first issue Issues that are well-suited for new contributors Package-novice table units
Projects
None yet
Development

No branches or pull requests

5 participants