Skip to content

Use the variable's time dimension in NCFileReader - #259

Open
ph-kev wants to merge 1 commit into
mainfrom
kp/time-name
Open

ph-kev wants to merge 1 commit into
mainfrom
kp/time-name

Conversation

@ph-kev

@ph-kev ph-kev commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

From the review in #258, it mentioned that the time dimension is accessed from a dataset instead of the variable of interest. This PR makes the same kind of change in #258 to get the right time dimension if there are two or more time dimensions in the dataset.

TODO

  • Update NEWS.md

Comment thread ext/nc_common.jl
# NCDatasets.jl uses CFTime.jl, which supports a time resolution of
# an attosecond, whereas Dates.DateTime only supports a time
# resolution of a millisecond.
return reinterpret.(Ref(Dates.DateTime), ds[time_dim][:])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why does this need to be reinterpret... this seems sketchy. Is there another way to do this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Base.reinterpret is defined for types in CFTime.jl and it is documented. See https://juliageo.org/CFTime.jl/stable/#Base.reinterpret

@ph-kev
ph-kev requested a review from imreddyTeja October 16, 2026 23:56

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants