liberfa/erfa

Minor documentation typo in atco13

jwoillez opened this issue · 7 comments

At https://github.com/liberfa/erfa/blob/master/src/atco13.c#L152
** eraAtioq quick ICRS to observed
should read
** eraAtioq quick CIRS to observed

timj commented

Good spot. Have you reported this upstream to SOFA? It is preferred that we do not have to track fixes in ERFA that cause divergence with upstream SOFA.

No, not yet. Would you recommend using the sofa@ukho.gov.uk E-mail given in http://www.iausofa.org/tandc.html?
If that's the one, I would also add it to the front page documentation of the github page. I can issue a PR for this.

timj commented

Yes, that email. It's a clear typo that I can't imagine will cause any issue. My personal preference is not to do a PR but to wait for the next SOFA release to include the fix. The more fixes that are done locally in ERFA the more pain is involved when bringing in SOFA updates. Not my call though. @eteq ?

timj commented

Or were you saying a PR to change the Readme? Sorry 😄

eteq commented

@timj @jwoillez - I'd actually say may as well put in a PR to ERFA fixing this and we can just merge it. It's probably not worth a release only for this, but it can take a long time for SOFA to integrate this sort of thing, and part of the point of ERFA is to have this in place in the meantime. I'm not too worried that SOFA will have a different solution, simply because it's quite literally only two letters different... And the scripts to re-generate from a new version of SOFA will just take in the new change if they do include it so nothing lost.

I sent the following E-mail to SOFA and submitted the two PRs above.

Dear SOFA,
I would like to draw your attention to a minor typo in the documentation of the atco13 function.
Please find the details in #29.
Sincerely,
-Julien

The issue was acknowledged by SOFA, with the addition of a similar typo in atio13, hence the PR #30 update (8d52bc0).