JuliaAstro / FITSIO.jl

Flexible Image Transport System (FITS) file support for Julia
http://juliaastro.org/FITSIO.jl/
MIT License
55 stars 29 forks source link

properly define get() instead of getkey() #184

Closed aplavin closed 1 year ago

aplavin commented 1 year ago

the current getkey() behavior should actually be get(): getkey in julia returns the key, while get returns the value

codecov[bot] commented 1 year ago

Codecov Report

Merging #184 (77e0b4f) into master (b272b28) will increase coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
+ Coverage   91.26%   91.29%   +0.02%     
==========================================
  Files           5        5              
  Lines         687      689       +2     
==========================================
+ Hits          627      629       +2     
  Misses         60       60              
Impacted Files Coverage Δ
src/FITSIO.jl 100.00% <ø> (ø)
src/header.jl 86.38% <100.00%> (+0.14%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

aplavin commented 1 year ago

I just wanted to avoid breaking changes: make it a deprecation for now, and then make a breaking release later when some other breaking changes are gathered. I strongly believe that the proper getkey fix has to be a breaking release (minor version), as it's a pretty common function changing behavior.

mileslucas commented 1 year ago

I have no qualms with a breaking release, I say go for it.

aplavin commented 1 year ago

Maybe do both? Add get & deprecate the current getkey in a non-breaking release, and then implement the proper getkey behavior in a breaking one. I've added another commit following your comments. After merging, you can register both versions: from the first and second commits. Both commits passed CI tests here.

mileslucas commented 1 year ago

That sounds like a good idea. Thanks for the help!