Skip to content

remove arrow support#2937

Open
verheem wants to merge 10 commits intomainfrom
remove-arrow-support
Open

remove arrow support#2937
verheem wants to merge 10 commits intomainfrom
remove-arrow-support

Conversation

@verheem
Copy link
Collaborator

@verheem verheem commented Feb 26, 2026

Fixes #2520
Fixes #2518

@verheem verheem marked this pull request as ready for review February 26, 2026 12:16
@verheem verheem marked this pull request as draft February 26, 2026 12:19
@verheem verheem marked this pull request as ready for review February 26, 2026 12:39
Copy link
Member

@visr visr left a comment

Choose a reason for hiding this comment

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

Left a few comments

Comment on lines 275 to 280
if !isempty(path)
name, ext = splitext(path)
if ext == ""
ext = config.results.format == "arrow" ? ".arrow" : ".nc"
path = string(name, ext)
path = string(name, ".nc")
end
end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if !isempty(path)
name, ext = splitext(path)
if ext == ""
ext = config.results.format == "arrow" ? ".arrow" : ".nc"
path = string(name, ext)
path = string(name, ".nc")
end
end

I think this whole block can go if you update const RESULTS_FILENAME to add the .nc back there now that it is not configurable.

Comment on lines +59 to +60
ds = df.set_index(["time", "node_id", "substance"]).to_xarray()
ds.to_netcdf(model.results_path / "basin_concentration_external.nc")
Copy link
Member

Choose a reason for hiding this comment

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

@evetion would this order need to be time, substance, node_id, the inverse of the Julia order for the results? ca97265

And the filename concentration.nc as discussed in #2933?

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.

Drop support for Arrow results Support compression for NetCDF results

2 participants