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

[FEATURE]: Rename spatial_avg.py to spatial.py #206

Closed
tomvothecoder opened this issue Mar 23, 2022 · 3 comments · Fixed by #207
Closed

[FEATURE]: Rename spatial_avg.py to spatial.py #206

tomvothecoder opened this issue Mar 23, 2022 · 3 comments · Fixed by #207
Assignees
Labels
type: enhancement New enhancement request
Projects

Comments

@tomvothecoder
Copy link
Collaborator

tomvothecoder commented Mar 23, 2022

Is your feature request related to a problem?

While working on #107, I noticed that I was adding temporal functions/methods to temporal_avg.py that weren't directly related to the TemporalAverageAccessor class. For example, the TemporalAverageAccessor.center_times() should actually just be a standalone function that lives in the .py module.

I ended up renaming the file from temporal_avg.py to temporal.py to store all temporal related functions and classes.

If we want to group functionality based on axes/dimensions, we should also rename spatial_avg.py to spatial.py.

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

@tomvothecoder tomvothecoder added the type: enhancement New enhancement request label Mar 23, 2022
@tomvothecoder tomvothecoder self-assigned this Mar 23, 2022
@tomvothecoder tomvothecoder added this to To do in v0.2.0 via automation Mar 23, 2022
@tomvothecoder
Copy link
Collaborator Author

Hey @pochedls, let me know what you think about this.

@pochedls
Copy link
Collaborator

Hey @pochedls, let me know what you think about this.

I like this a lot.

@durack1
Copy link
Collaborator

durack1 commented Mar 23, 2022

@tomvothecoder I think this is ideal, you may want median, min, max, ..., operations in addition to average so more general axes/spatial and time time_coordinate/temporal is a smart move. Just thinking aloud, I wonder if vertical would be folded into axes, or would be its own class?

@pochedls has the arbitrary index discussion come up anywhere, so what I mean is captured in CDAT/cdat#1288 - this very well maybe already a core part of xarray

v0.2.0 automation moved this from To do to Done Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New enhancement request
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants