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

CompatHelper: bump compat for NCDatasets to 0.13, (keep existing compat) #305

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 5, 2023

This pull request changes the compat entry for the NCDatasets package from 0.12.11 to 0.12.11, 0.13.
This keeps the compat entries for earlier versions.

Note: I have not tested your package with this new compat entry.
It is your responsibility to make sure that your package tests pass before you merge this pull request.

@visr visr force-pushed the compathelper/new_version/2023-10-05-11-53-02-577-02313901943 branch from 74ab153 to c592705 Compare October 5, 2023 11:53
@visr
Copy link
Member

visr commented Oct 5, 2023

This will need some updates to the code. At least Wflow is affected by this change:

  • ncvar2D[:] flattens the data in the 2D NetCDF variable ncvar2D. To read the full array one need to use ncvar2D[:,:] or Array(ncvar2D) (similarly for 3D, 4D... arrays).

Probably it's not worth supporting both 0.12 and 0.13, so I think we should drop 0.12 support after these issues are addressed.

https://github.com/Alexander-Barth/NCDatasets.jl/releases/tag/v0.13.0

@JoostBuitink
Copy link
Contributor

I took a a quick look, and it appears that there are hardly (none) changes required to make our code compatible with 0.13. Only one line in the tests broke, but all other code looks to be fine. I still decided to drop 0.12, to be clear that we are compliant with 0.13

@JoostBuitink JoostBuitink requested a review from verseve November 9, 2023 16:04
@visr
Copy link
Member

visr commented Nov 9, 2023

What about lines like flood_depths = Float.(nc["flood_depth"][:]), are those variables 1D or does it just not matter that we flatten them?

@JoostBuitink
Copy link
Contributor

Those are 1d (as they are a dimension in the netcdf)!

Copy link
Contributor

@verseve verseve left a comment

Choose a reason for hiding this comment

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

Thanks for checking Joost, looks good to me!

@JoostBuitink JoostBuitink merged commit 91a8572 into master Nov 16, 2023
11 checks passed
@JoostBuitink JoostBuitink deleted the compathelper/new_version/2023-10-05-11-53-02-577-02313901943 branch November 16, 2023 14:27
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.

3 participants