More informative changelogs for breaking releases

[Sbozzolo]

It would be very useful to have informative release notes for breaking releases with clear instructions on how to move to the new version (more like in version 0.13.0).

For example, in 0.14.0 we have

Many functions and types not specific to NetCDF files (multi-files, views, and load by value) have been moved to CommonDataModel.jl

It sounds something that might affect me, but I don't understand what this really means. I tried looking at the commits, but that's something I can easily pick up by skimming throw the logs.

(In general, I found that 0.14.0 is incompatible with my code, but I couldn't understand why from the release notes)

Ideally, I think the release notes should:

• split and highlight breaking changes from non-breaking changes (when applicable), and maybe add a heading to draw attention to the fact that there's breaking change.
• provide clear information on how to perform the transition with specific examples.

Adding examples allows developers to more easily identify patterns that have to be changed and learn what needs to be done.

Thank you!

[Sbozzolo]

(The change that broke my code is that .dim behaved like an Array, and now it behaves like a Dict. I used NCDatasets.dimnames to restore compatibility).

EDIT: It also changes the .attrib field. Now Dict(ds[var].attrib) apparently also contains Symbols (typeof(Dict(ds[var].attrib)) = Dict{Union{AbstractString, Symbol}, Any} even though there should only be Strings).

[Alexander-Barth]

Don't forget that this package is a volunteer driven project. If you (or somebody else) have the time to help with testing and writing example for the release notes that would be more than welcome.

As in julia, what is considered as public API and covered by semantic versioning is what documented (https://docs.julialang.org/en/v1/manual/faq/#How-does-Julia-define-its-public-API?).

If there is anything in the documentation, that is not (or no longer) accurate, please let me know. In the past, also some user have contributed examples in the documentation for some feature they depend on. I think that is a great way to give additional stability to the features that are important to you.

But not every observable feature of NCDatasets is considered as public API.

The change that broke my code is that .dim behaved like an Array, and now it behaves like a Dict. I used NCDatasets.dimnames to restore compatibility

Can you elaborate what was Array-like in .dim ? Is it mentioned in the documentation?

In NCDatasets 0.13 I have:

julia> fname = "/tmp/foo.nc"
"/tmp/foo.nc"

julia> ds = NCDataset("/tmp/foo.nc","c"); defVar(ds,"data",randn(4,3),("lon","lat"),attrib=Dict("foo" => "bar")); close(ds);
julia> ds = NCDataset(fname);
julia> ds.dim[1]
ERROR: MethodError: no method matching getindex(::NCDatasets.Dimensions{NCDataset{Nothing}}, ::Int64)

Closest candidates are:
  getindex(::NCDatasets.Dimensions, ::AbstractString)
   @ NCDatasets ~/.julia/dev/NCDatasets/src/dimensions.jl:23

Stacktrace:
 [1] top-level scope
   @ REPL[4]:1

julia> ds.dim isa AbstractArray
false

julia> collect(ds.dim)
2-element Vector{Any}:
 "lon" => 4
 "lat" => 3

EDIT: It also changes the .attrib field. Now Dict(ds[var].attrib) apparently also contains Symbols (typeof(Dict(ds[var].attrib)) = Dict{Union{AbstractString, Symbol}, Any} even though there should only be Strings).

Also in NCDatasets 0.13, you can index .attrib with symbols, e.g.

ds[:data].attrib[:foo]

Let me know if there is something in the documentation that misled you.

In any case, please provide minimal reproducible example. It helps me a lot to find what is the issue.

Adding examples allows developers to more easily identify patterns that have to be changed and learn what needs to be done.

I really agree that this would be useful, but the free time that I can dedicate to this is limited.

[Sbozzolo]

Don't forget that this package is a volunteer driven project. If you (or somebody else) have the time to help with testing and writing example for the release notes that would be more than welcome.

And, I really appreciate your work! Please, don't take this issue as me demanding anything, I have full respect for your time and effort and didn't mean to sound ungrateful.

Feel free to ping me before a release if you want me to check compatibility with some CliMA packages. I would be more than happy to test the new version out and sort out potential problems.

Can you elaborate what was Array-like in .dim ? Is it mentioned in the documentation?

Your comment made me have a second look, and indeed I shouldn't have said that is Array-like. From the docs I can clearly see that is Dict-like.

The code that stopped working was map(fun, ds.dim):

ERROR: map is not defined on dictionaries
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] map(f::Function, #unused#::CommonDataModel.Dimensions{NCDataset{Nothing}})
   @ Base ./abstractarray.jl:3293
 [3] top-level scope
   @ REPL[5]:1

Also in NCDatasets 0.13, you can index .attrib with symbols, e.g.

ds[:data].attrib[:foo]
Let me know if there is something in the documentation that misled you.

I didn't know Symbols could be used for indexing. I just searched .attrib and I couldn't see an example where Symbols are used in the code (except in a unit test). If this is the case, it would be useful to add such examples (I do see that the type is SymbolOrString in the docstring in Base.getindex).

That said, keys(ds[var].attrib) returns a Vector{String}. collect(ds[var].attrib) also returned a Vector of Pair{String, String} in 0.13. I used this to transform the attributes in actual typed Dicts.

[Alexander-Barth]

Thank for providing this additional information! I will try to ping you when I have a "major" change this this (but sadly, I am sometimes a bit in a rush, if I need to release to code before a workshop...)

Yes, I should add the fact that one can use Symbols as keys to the documentation. Good point!

Thanks to your example, I have now a better idea what happened. Previously, ds.dim was Dict-like, but not a subtype of AbstractDict. The call to map(fun, ds.dim), called this function:

https://github.com/JuliaLang/julia/blob/631e7ef6ffee377b87b4a156cc41bb25305a6c31/base/abstractarray.jl#L3335

At the next line of the same file, you see that map is explicitly disallowed for AbstractDicts.
In CommonDataModel 0.3, structures like ds.dim are now a suptype of AbstractDicts. I did not anticipate that this would break user code.

I already added a summary of this info to the release notes of 0.14 as it might help others.

[Sbozzolo]

I also found this:

NCDatasets.CFVariable no longer exists (it is now CommonDataModel.CFVariable), but the constructor for nc = NCDataset(var) is not available when var is a CommonDataModel.CFVariable:

  MethodError: no method matching NCDataset(::CommonDataModel.CFVariable{Float64, 1, NCDatasets.Variable{Float64, 1, NCDataset{Nothing}}, CommonDataModel.Attributes{NCDatasets.Variable{Float64, 1, NCDataset{Nothing}}}, NamedTuple{(:fillvalue, :missing_values, :scale_factor, :add_offset, :calendar, :time_origin, :time_factor), Tuple{Nothing, Tuple{}, Vararg{Nothing, 5}}}})
  
  Closest candidates are:
    NCDataset(::TDS, ::Int32, ::Bool, ::Ref{Bool}, ::Dict{String, String}) where TDS
     @ NCDatasets ~/.julia/packages/NCDatasets/w3iru/src/types.jl:29
    NCDataset(::CommonDataModel.MFCFVariable)
     @ NCDatasets ~/.julia/packages/NCDatasets/w3iru/src/multifile.jl:67
    NCDataset(::Function, ::DeferDataset)
     @ NCDatasets ~/.julia/packages/NCDatasets/w3iru/src/defer.jl:13

I can probably open a PR if it is just a matter of adding a type annotation for CDM.CFVariable at line 25 of variables.jl. Or, you can add this to the release notes:

NCDatasets.CFVariable was moved to CommonDataModel.CFVariable. NCDatasets.NCDataset(var::NCDatasets.CFVariable) is no longer supported, use NCDatasets.dataset(var::CommonDataModel.CFVariable) instead.

[Alexander-Barth]

thanks, I add it to the release notes.