Fix NCDatasets extension for Julia 1.12#127
Conversation
Replace __init__ + @eval pattern with a direct import and compile-time method definition for default_output_attributes. The old pattern broke incremental compilation on Julia 1.12 because eval into a closed module is no longer allowed. Requires Oceananigans >= 0.106.4 (CliMA/Oceananigans.jl#5469). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ext/ClimaSeaIceNCDatasetsExt.jl
Outdated
| function default_output_attributes(model::SeaIceModel) | ||
| velocity_attrs = default_horizontal_velocity_attributes(model.grid) | ||
| tracer_attrs = default_sea_ice_attributes() | ||
| return merge(velocity_attrs, tracer_attrs) | ||
| end |
There was a problem hiding this comment.
Why is this method defined here? default_output_attributes is from Oceananigans.OutputWriters, and SeaIceModel is defined in the main package, I don't see why it should be defined in the extension with NCDatasets
There was a problem hiding this comment.
Ah, it was here because before CliMA/Oceananigans.jl#5469 you couldn't extend it outside this NCDatasets extension. But still, with that PR I believe this method should be defined in the main package, not here
ext/ClimaSeaIceNCDatasetsExt.jl
Outdated
| using Oceananigans | ||
| using ClimaSeaIce | ||
|
|
||
| import Oceananigans.OutputWriters: default_output_attributes |
|
|
I don't think it's a duplicate, it's a different approach to address the same issue. And I think this PR is better than #126 because it doesn't use |
|
Ah sorry, I just looked at the title.
ah I see |
Address review feedback: move default_output_attributes(::SeaIceModel) and helper functions from the NCDatasets extension to the main ClimaSeaIce module. With Oceananigans#5469, default_output_attributes is a stub in Oceananigans.OutputWriters, so the method can be defined at compile time without needing the NCDatasets extension. Uses qualified name instead of import. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The extension has no code left after moving default_output_attributes to the main module. Remove the extension file, weakdep, and extension entry from Project.toml. Keep NCDatasets in [extras] for tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
I also had just looked at the title 😅 without reading the comments. I ll reopen it |
3 participants
Summary
__init__+@evalpattern with a directimport+ compile-time method definition fordefault_output_attributesMotivation
The
@evalin__init__breaks incremental compilation on Julia 1.12 with the error:This caused
NumericalEarth(and any package loading both ClimaSeaIce and NCDatasets) to fail precompilation.Dependencies
Requires CliMA/Oceananigans.jl#5469 — which moves
default_output_attributesfrom the NCDatasets extension to a stub in the main Oceananigans module, so downstream extensions can extend it at compile time.Test plan
🤖 Generated with Claude Code