Skip to content

Add doc strings to constants and add SOFA names#36

Open
david-macmahon wants to merge 1 commit intoJuliaAstro:mainfrom
david-macmahon:constant-branch
Open

Add doc strings to constants and add SOFA names#36
david-macmahon wants to merge 1 commit intoJuliaAstro:mainfrom
david-macmahon:constant-branch

Conversation

@david-macmahon
Copy link
Contributor

Add doc strings to constants defined in src/constants.jl. Also add missing SOFA/ERFA names for constants. Even though they are somewhat cryptically named they are useful when porting code that uses them.

Add doc strings to constants defined in src/constants.jl.  Also add
missing SOFA/ERFA names for constants.  Even though they are somewhat
cryptically named they are useful when porting code that uses them.
@codecov
Copy link

codecov bot commented Mar 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.75%. Comparing base (23eced0) to head (1241556).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #36   +/-   ##
=======================================
  Coverage   92.75%   92.75%           
=======================================
  Files          24       24           
  Lines        1685     1685           
=======================================
  Hits         1563     1563           
  Misses        122      122           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@barrettp
Copy link
Member

barrettp commented Mar 1, 2026

constants.jl is primarily for the Astrometry.jl package, not the SOFA subpackage, so there should be no reference to it in the constants.jl file. You are welcome to add those definitions to a constants file in the SOFA subpackage.

@david-macmahon
Copy link
Contributor Author

The src/constants.jl file is included by both src/Astrometry.jl and src/sofa.jl. Shall I create a new file src/sofa_constants.jl and include that in src/sofa.jl instead of src/constants.jl? Or put only the "missing" ones in src/sofa_constants.jl and include it in src/sofa.jl in addition to src/constants.jl?

@barrettp
Copy link
Member

barrettp commented Mar 2, 2026

At this stage, it would probably be best to make the SOFA submodule completely independent of the Astrometry module by duplicating all of the constant files and placing them in the SOFA submodule.

It would be even better to replace the wrapped SOFA.jl package with the pure Julia SOFA submodule. We should contact the owner of the SOFA.jl package to see if they would be interested in such a change.

@david-macmahon
Copy link
Contributor Author

What do you mean by "replace the wrapped SOFA.jl package with the pure Julia SOFA submodule"? Isn't this already pure Julia? Maybe I misunderstand things. I thought Astrometry.jl is intended to be a pure Julia version of SOFA.

@barrettp
Copy link
Member

barrettp commented Mar 2, 2026

I am suggesting that the wrapped C library of either SOFA.jl or ERFA.jl be replaced by the pure Julia SOFA submodule of Astrometry.jl, now that a pure Julia version exists. In other words, yank the SOFA submodule out of Astrometry.jl and put it in SOFA.jl (or ERFA.jl), so that the two modules are separate and distinct.

@david-macmahon
Copy link
Contributor Author

What would be left in Astrometry.jl? I guess I always thought of Astrometry.jl as being "just" a pure Julia implementation of SOFA rather than being a user (and creator) of a pure Julia SOFA while also providing higher level functionality.

@barrettp
Copy link
Member

barrettp commented Mar 2, 2026

Astrometry.jl is meant to be simplified, yet complete, redesign of the SOFA functions. The SOFA submodule was added to check the accuracy of the Astrometry functions, but was not intended to be the primary module. It just happens to be now because not much development has been done on the main module. This is mainly because there is no TimeScales.jl and ReferenceFrames.jl packages. In other words AstroTime.jl is incomplete in my opinion, because the reference frames are implied, but not explicit. The goal is to develop a holistic package. This is the reason why I want to keep the SOFA submodule independent of the Astrometry module.

@david-macmahon
Copy link
Contributor Author

I think it would be great to have a standalone pure Julia implementation of the SOFA/ERFA library. I'm not sure the best way to package that, but it's probably better discussed in an issue rather than this PR. 😅

@tamasgal
Copy link
Collaborator

tamasgal commented Mar 3, 2026

I agree that we should discuss this separately ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants