Expanding "operation" keyword #119
Conversation
| @@ -338,10 +338,10 @@ def map(*layers, | |||
| ndim=ndim) | |||
|
|
|||
| # Apply operation along depth | |||
| binned = getattr(binned, operation)(axis=1) | |||
There was a problem hiding this comment.
I'm starting to think that operation shouldn't be a string but just a callable. Then you would pass whatever function you want, it could even be a custom function.
There was a problem hiding this comment.
How would the map function handle units in that case?
There was a problem hiding this comment.
I don't think we are handling the units well at the moment, so maybe we can come up with a better idea?
We are only doing something special if the operation is a sum.
See also #72
There was a problem hiding this comment.
Maybe we should just have a special top-level function for computing column densities, instead of relying on a special case of the thick map.
It would just be a thin wrapper around the map function, but this would make what happens to the units less ambiguous/hidden?
There was a problem hiding this comment.
I can give it a go in the coming weeks.
I'll close this PR and open a new one if that's okay with you.
There was a problem hiding this comment.
Alternatively, we can always merge this one for now and improve the mechanism later, since this is not breaking anything, just making you able to use nansum if you wish.
There was a problem hiding this comment.
Sure, let's do that in the meantime.
|
|
||
| # Handle thick maps | ||
| if thick and (operation == "sum"): | ||
| if thick and ((operation == "sum") or (operation == "nansum")): |
There was a problem hiding this comment.
You will need to update the docstring to the map function though.
src/osyris/plot/map.py
Outdated
| not ``None``. Possible values are ``'sum'``, ``'mean'``, ``'min'``, and | ||
| ``'max'``. Default is ``'sum'``. | ||
| not ``None``. Possible values are ``'sum'``, ``'nansum'``, | ||
| ``'mean'``, ``'min'``, and ``'max'``. Default is ``'sum'``. |
There was a problem hiding this comment.
You may want to add nanmean, nanmin and nanmax while you're at it?
This allows the user to call any numpy operation when performing thick maps. It is particularly useful in star formation when doing column densities of cells belonging to the disk only, as it requires us to call
nansuminstead ofsumsince the non-disk cells are masked. Below is an example of this: