Skip to content

Conversation

@marqh
Copy link
Member

@marqh marqh commented Oct 10, 2016

possible fix for netcdf4-python behaviour change

in pp_rules date time calculations used to return floats, and still do in python2 but not in python3

return epoch_hours

this manifested as a change in a doctest, but looks like a widespread issue where cubes are created using
hours_from_t1_to_t2
and such like

hours_from_t1_to_t2 = t2_epoch_hours - t1_epoch_hours

this work around may fix the problem

@marqh
Copy link
Member Author

marqh commented Oct 10, 2016

i think this should be evaluated against an unpinned matplotlib (thus an unpinned netcdf4-python), for python2 and python3, away from travis before the change is merged as it is in preparation for unpinning matplotlib

@marqh
Copy link
Member Author

marqh commented Oct 11, 2016

i think this should be evaluated against an unpinned matplotlib (thus an unpinned netcdf4-python), for python2 and python3, away from travis before the change is merged as it is in preparation for unpinning matplotlib

marqh#26

I believe this supports that this change is valid, though it is very noisy due to outstanding python3 failures

if t.minute == 0 and t.second == 0:
epoch_hours = round(epoch_hours)
return epoch_hours
return float(epoch_hours)
Copy link
Member

@bjlittle bjlittle Oct 12, 2016

Choose a reason for hiding this comment

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

@marqh A comment explaining the use of the cast would be useful here ...

@bjlittle bjlittle self-assigned this Oct 12, 2016
@bjlittle bjlittle added this to the v1.11 milestone Oct 12, 2016
# numpy.float64. The behaviour of round is to recast this to an
# int, which is not the desired behaviour for PP files.
# So, cast the answer to numpy.float_ to be safe.
epoch_hours = np.float_(epoch_hours_unit.date2num(t))
Copy link
Member

Choose a reason for hiding this comment

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

@marqh OMGosh ... thanks for digging!

@bjlittle bjlittle merged commit b95fccf into SciTools:master Oct 12, 2016
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