Add spectral flux equivalency property to SpectralQuantity#10263
Add spectral flux equivalency property to SpectralQuantity#10263eteq wants to merge 3 commits intoastropy:mainfrom
Conversation
pllim
left a comment
There was a problem hiding this comment.
I changed the label to "affects-dev" because it is more accurate from your description.
Does this needs to be advertised under https://docs.astropy.org/en/latest/units/equivalencies.html ?
| def test_flux_density(self): | ||
| sq1 = SpectralQuantity([10, 20, 30] * u.AA) | ||
|
|
||
| f_lamb = [1, 1, 1] * u.erg / u.cm**2 / u.s / u.AA |
There was a problem hiding this comment.
| f_lamb = [1, 1, 1] * u.erg / u.cm**2 / u.s / u.AA | |
| f_lamb = [1, 1, 1] * (u.erg / u.cm**2 / u.s / u.AA) |
|
|
||
| to_fnu = f_lamb.to(u.erg / u.cm**2 / u.s / u.Hz, | ||
| equivalencies=sq1.spectral_density_equivalency) | ||
| assert_quantity_allclose(to_fnu, [1, 2**2, 3**2]*(3.33e-17 * u.erg / u.cm**2 / u.s / u.Hz), rtol=.01) |
There was a problem hiding this comment.
Nitpick:
| assert_quantity_allclose(to_fnu, [1, 2**2, 3**2]*(3.33e-17 * u.erg / u.cm**2 / u.s / u.Hz), rtol=.01) | |
| assert_quantity_allclose(to_fnu, [1, 2**2, 3**2] * (3.33e-17 * (u.erg / u.cm**2 / u.s / u.Hz)), rtol=.01) |
Also, why is the rtol so high?
|
Seeing it again, on its own, I'm still not really convinced, as it is so easy to do separately and would seem to below in |
|
Removing the milestone as it is not urgent. |
|
Hi humans 👋 - this pull request hasn't had any new commits for approximately 5 months. I plan to close this in a month if the pull request doesn't have any new commits by then. In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock. If you believe I commented on this pull request incorrectly, please report this here. |
I agree. Having relevant equivalencies easily accessible seems a nice feature. Something similar for dimensionless angles might be worthwhile on |
|
Hi humans 👋 - this pull request hasn't had any new commits for approximately 1 year, 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then. In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary. If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock. If you believe I commented on this pull request incorrectly, please report this here. |
|
I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks! If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here. |
This is a follow-on from #10185 - in #10185 (review) I suggested this idea, issued it as a PR to @astrofrog's #10185 branch via astrofrog#100, but then in #10185 (comment) @mhvk rightly brought up some questions, so I've re-worked it as a PR against master since #10185 was ready to merge. So this has already gotten some review from @astrofrog, but it would be good to hear what @mhvk thinks.
(I've labeled in
no-changelog-entry-neededon the theory we might get it in for 4.1, but that will have to be revised if it gets pushed to 4.2).