NetCDFOutputWriter should have mode = "c" as default?

[navidcy]

I was reading the NetCDFOutputWriter docstring and I got confused. Why mode = nothing is set as default in NetCDFOutputWriter

Oceananigans.jl/src/OutputWriters/netcdf_output_writer.jl

Line 289 in 6ceeb01

mode = nothing,

and then straight after is converted to mode = "c"

Oceananigans.jl/src/OutputWriters/netcdf_output_writer.jl

Line 301 in 6ceeb01

isnothing(mode) && (mode = "c")

?

Why not have mode = "c" the default?

[glwagner]

The default depends on whether the file already exists or not? Just above that...

Oceananigans.jl/src/OutputWriters/netcdf_output_writer.jl

Lines 293 to 298 in 6ceeb01

	if isfile(filepath) && isnothing(mode)
	@warn "$filepath already exists but no NetCDFOutputWriter mode was explicitly specified. " *
	"Will default to mode = \"a\" to append to existing file. You might experience errors " *
	"when writing output if the existing file belonged to a different simulation!"
	mode = "a"
	end

[glwagner]

It's all a little weird though I agree...

Partly I think we're in a much better position to figure out whether we want to destroy an existing file or append to a new one when we call run!. At the time the output writers are created, we don't know if someone is pickup=true or if iteration != 0, etc. I think there's an issue about this...

[glwagner]

Something about "initializing" an output writer only after we call run!. Basically we can just move the whole constructor to some function initialize_output_writer! that only get's called when a simulation is "initialized".

[glwagner]

Resolved, right @tomchor ?

[tomchor]

Kinda. The current behavior is this

Oceananigans.jl/src/OutputWriters/netcdf_output_writer.jl

Lines 302 to 308 in 470fd11

	if isnothing(overwrite_existing)
	if isfile(filepath)
	overwrite_existing = false
	else
	overwrite_existing = true
	end
	else

So the default is similar to what it was when this issue was posted, although I think it's formulated a little clearer now.

I'm okay with this and also okay with closing this issue, but I'm not sure everyone else feels this way. You mentioned at some point that we could move this part to after run!() is called and I don't think we discussed that.

[glwagner]

Oh I see... you're right, we should leave this open.

[glwagner]

I'm closing this issue because I'm judging that it's not of current, timely relevance to Oceananigans development. If you would like to make it a higher priority or if you think the issue was closed in error please feel free to re-open.