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

Bnb/era edits #146

Merged
merged 10 commits into from
Apr 7, 2023
Merged

Bnb/era edits #146

merged 10 commits into from
Apr 7, 2023

Conversation

bnb32
Copy link
Collaborator

@bnb32 bnb32 commented Apr 6, 2023

DataHandlerNCforERA addition. resolved issues with pandas version update. unpinned xarray, netcdf, sphinx. Incremented version. Removed support for python 3.7. Added python 3.10.

@bnb32 bnb32 force-pushed the bnb/era_edits branch 2 times, most recently from db8398a to 9f09161 Compare April 6, 2023 15:37
@bnb32 bnb32 force-pushed the bnb/era_edits branch 4 times, most recently from e7ea76c to 242fad2 Compare April 6, 2023 17:50
@bnb32 bnb32 force-pushed the bnb/era_edits branch 3 times, most recently from abd02cd to c4586f1 Compare April 6, 2023 20:09
@bnb32 bnb32 requested a review from grantbuster April 6, 2023 21:59
Copy link
Member

@grantbuster grantbuster left a comment

Choose a reason for hiding this comment

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

looks good, minor comments

Parameters
----------
filepath : str
Path to the file
Copy link
Member

Choose a reason for hiding this comment

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

.nc file only? Move this into the NC specific class?

if len(time_key) > 0:
return time_key[0]
else:
return 'time'
Copy link
Member

Choose a reason for hiding this comment

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

why is this a class method instead of static? Can you make a utility function for this since the same functionality is needed in post processing above? if not, move to the nc specific classes? Can you update the docstring to clarify that this is just for .nc?

if len(data['orog'].dims) == 3:
hgt = data['orog'][(0,) + tuple(raster_index)]
else:
hgt = data['orog'][tuple(raster_index)]
Copy link
Member

Choose a reason for hiding this comment

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

These interp utilities are nearing the point where we should move them all into an interp utilities class. Complexity for this method alone is quite high, and the utilities.py file alone is massive

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah agreed. Moved to new class.

sup3r/utilities/utilities.py Outdated Show resolved Hide resolved
@bnb32 bnb32 merged commit ea3dc69 into main Apr 7, 2023
@bnb32 bnb32 deleted the bnb/era_edits branch April 7, 2023 17:41
github-actions bot pushed a commit that referenced this pull request Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants