HDF5.jl icon indicating copy to clipboard operation
HDF5.jl copied to clipboard

Deprecate old filter syntax for v0.16

Open musm opened this issue 3 years ago • 7 comments

I'd like to finalize the v0.16 release, but there's one final issue I'd like to settle, which is whether we should deprecate the old property syntax for setting filters, in favor of the new extensible filter interface. I don't see there being a better/more robust extensible way to implement properties to permit such syntax, thus we should deprecate them for consistency.

In any case, the only argument for keeping it is convenience .

musm avatar Jan 21 '22 22:01 musm

@mkitti I think you mentioned you were in favor of this change, is your thinking still the same for v0.16? Alternatively we could keep in in v0.16 and deprecate the syntax for v0.17, but it feels like since we made all the filter changes in v0.16 it might be more convenient to do it in this release.

musm avatar Jan 21 '22 22:01 musm

I think deprecation is fine. The old method still works and frankly I don't see a good reason to completely do away with it at the moment or in the near future. By v0.17 you would be in the clear to remove it completely, but I would not recommend it unless it was getting in the way of something else.

mkitti avatar Jan 21 '22 22:01 mkitti

If I understand your suggestion. It's to deprecate the method except keep the deprecation for at least 2 minor release cycles?

musm avatar Jan 21 '22 22:01 musm

I would not plan on removing it at the moment. I would state in the release notes that it could be removed in two release cycles, so we would be free to do so if needed. I currently do not see a strong need to remove it now or in the future, and I would not recommend on actually removing until there is such a need.

mkitti avatar Jan 21 '22 22:01 mkitti

I would state in the release notes that it could be removed in two release cycles, so we would be free to do so if needed. I currently do not see a strong need to remove it now or in the future, and I would not recommend on actually removing until there is such a need.

If we have that plan, I think that would necessitate at least a deprecation warning in the current or next cycle stating it's potential deprecation in the next one-two cycles. I don't think just adding the note in the release notes would be sufficient, since it would be easy to miss and make upgrading the deprecation harder.

musm avatar Jan 25 '22 15:01 musm

All deprecation means is that this is old syntax and that you should migrate to a new API. It does not imply imminent removal. It merely indicates future removal and discourages use of that API.

Java applets for examples have been deprecated in Java 9 in 2016 and were not removed until Java 11 was released in late 2018.

I would tend to deprecate liberally. This provides clear guidance about what API is preferred and will be supported in the future an what would not be.

I would remove API only when it becomes an undue burden because this actually breaks code. If we can map the old API to the new API, then it might be worth keeping it around for a while, or perhaps moving it to a package in a similar fashion as to how Compat.jl works. Notice that people only generally complain when you remove API.

mkitti avatar Jan 25 '22 17:01 mkitti

It sounds like we are on the same page. I wasn't advocating we remove it imminently. And of course, as you mention, it would be technically ok to do so by v0.17, but we probably should wait longer. For v0.16 we should add the deprecation notice to make it clear that the syntax may be removed and the new filter API is preferred.

musm avatar Jan 25 '22 17:01 musm