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

Add support to specify key type and always decode level as float #210

Merged
merged 7 commits into from
Apr 8, 2021

Conversation

alexamici
Copy link
Contributor

@alexamici alexamici commented Mar 11, 2021

With this change you can optionally specify the key type when using Message.__getitem__ like:

>>> msg["level:float"]
1.0
>>> msg["level:int"]
1
>>> msg["level"]
1

Then the level key is decoded as float in order to close #195 . Note that this is a significant user-visible change but I think it is the best default.

@alexamici alexamici requested a review from iainrussell March 11, 2021 20:38
@iainrussell
Copy link
Member

Sorry @alexamici I missed this PR. It looks good to me, but it would look even better if there were some tests, if I may be so cheeky :)

@alexamici
Copy link
Contributor Author

I'm now hesitant to add this feature to cfgrib as it really looks like a lower level functionality. @shahramn do you have a way to specify the target data type in the key, like msg["level:float"]. Would you be interested in addign this functionality to eccodes-python?

@shahramn
Copy link
Collaborator

shahramn commented Apr 6, 2021

I'm now hesitant to add this feature to cfgrib as it really looks like a lower level functionality. @shahramn do you have a way to specify the target data type in the key, like msg["level:float"]. Would you be interested in addign this functionality to eccodes-python?

We already have a way of passing in the type information. For example:

% python3
>>> import eccodes
>>> f = open(input, 'rb')
>>> gid = eccodes.codes_grib_new_from_file(f)
>>> print( eccodes.codes_get(gid, 'level') )
65
>>> print( eccodes.codes_get(gid, 'level', float) )
65.4
>>> print( eccodes.codes_get_double(gid, 'level') )
65.4

@codecov-io
Copy link

codecov-io commented Apr 7, 2021

Codecov Report

Merging #210 (c253090) into stable/0.9.9.x (919a513) will decrease coverage by 0.08%.
The diff coverage is 80.00%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           stable/0.9.9.x     #210      +/-   ##
==================================================
- Coverage           92.29%   92.20%   -0.09%     
==================================================
  Files                  25       25              
  Lines                1714     1720       +6     
  Branches              201      202       +1     
==================================================
+ Hits                 1582     1586       +4     
- Misses                110      111       +1     
- Partials               22       23       +1     
Impacted Files Coverage Δ
cfgrib/messages.py 93.16% <66.66%> (-0.61%) ⬇️
cfgrib/dataset.py 98.93% <100.00%> (+<0.01%) ⬆️

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 919a513...c253090. Read the comment docs.

@alexamici
Copy link
Contributor Author

@iainrussell I added the tests for the generic functionality. This is ready to merge for me.

Copy link
Member

@iainrussell iainrussell 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 to me, great that eccodes has such a nice interface to allow this to work so conveniently!

@alexamici alexamici merged commit e8301ae into stable/0.9.9.x Apr 8, 2021
@alexamici alexamici deleted the float-levels branch July 29, 2021 17:49
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.

4 participants