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

Rename spatial file and class #207

Merged
merged 1 commit into from
Mar 24, 2022
Merged

Conversation

tomvothecoder
Copy link
Collaborator

@tomvothecoder tomvothecoder commented Mar 23, 2022

Description

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

If applicable:

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass with my changes (locally and CI/CD build)
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected) -- won't release a new major version since the package is still in beta stage

@tomvothecoder tomvothecoder added type: enhancement New enhancement request Priority: Medium breaking-change A breaking change, intended for a major release. labels Mar 23, 2022
@tomvothecoder tomvothecoder self-assigned this Mar 23, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 23, 2022

Codecov Report

Merging #207 (5730e24) into main (201a575) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              main      #207    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            8         9     +1     
  Lines          381       742   +361     
==========================================
+ Hits           381       742   +361     
Impacted Files Coverage Δ
xcdat/__init__.py 100.00% <100.00%> (ø)
xcdat/axis.py 100.00% <100.00%> (ø)
xcdat/bounds.py 100.00% <100.00%> (ø)
xcdat/dataset.py 100.00% <100.00%> (ø)
xcdat/spatial.py 100.00% <100.00%> (ø)
xcdat/temporal.py 100.00% <100.00%> (ø)
xcdat/utils.py 100.00% <100.00%> (ø)
xcdat/xcdat.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d75704e...5730e24. Read the comment docs.

@tomvothecoder tomvothecoder requested a review from pochedls March 23, 2022 22:10
@tomvothecoder
Copy link
Collaborator Author

Hi @pochedls, the diff in this PR is pretty small, but I wanted to see if you had any comments first before I merge it.

Copy link
Collaborator

@pochedls pochedls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't actually pull this code and test it, but I did look through it and I approve. I didn't Immediately understand some of the superficial changes, e.g., from comment "Access spatial averaging method" to "Call spatial averaging method" but I assume there is a consistency reason for the change.

Overall, I like spatial better than spatial_avg

@tomvothecoder
Copy link
Collaborator Author

tomvothecoder commented Mar 24, 2022

I didn't Immediately understand some of the superficial changes, e.g., from comment "Access spatial averaging method" to "Call spatial averaging method" but I assume there is a consistency reason for the change.

Yeah, those are just minor changes to align with programmatic terms. I'll comment next time on those types of changes since they aren't obvious.

Overall, I like spatial better than spatial_avg

Sounds good! I will merge this PR, thanks.

- Rename `spatial_avg.py` to `spatial.py`
- Rename `SpatialAverageAccessor` to `SpatialAccessor`
- These changes enable future extensibility in case methods or functions related to spatial axes are integrated into the xcdat package.
@tomvothecoder tomvothecoder force-pushed the feature/206-rename-spatial branch from 5730e24 to 22a5f7a Compare March 24, 2022 15:20
@tomvothecoder tomvothecoder merged commit d44dafb into main Mar 24, 2022
@tomvothecoder tomvothecoder deleted the feature/206-rename-spatial branch March 24, 2022 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change A breaking change, intended for a major release. type: enhancement New enhancement request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE]: Rename spatial_avg.py to spatial.py
3 participants