-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement spk parser #4
Conversation
a5ab074
to
2d0ae4d
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4 +/- ##
==========================================
+ Coverage 92.40% 93.12% +0.71%
==========================================
Files 15 16 +1
Lines 5053 5698 +645
==========================================
+ Hits 4669 5306 +637
- Misses 384 392 +8
☔ View full report in Codecov by Sentry. |
I think I would prefer getting rid of the |
I asked @helgee what user-facing API should I implement, he said the following:
|
2d0ae4d
to
470c5f8
Compare
bf0d6ac
to
3b70ce2
Compare
@helgee, we might need to do a bit of a rewrite on the NAIF object stuff. I need a simple way to do the mapping between the ID and the correct object. What I have now looks like:
But this is annoying, because now just to return the trait stuff we have to add a heap-allocated
So I would see this move closer to an enum, like below, where everything is stack allocated and doesn't involve funny stuff.
|
A design decision is to keep the NAIF body IDs as part of the structure and not do the conversion to the trait object at this level. Furthermore, we want to keep using the bodies as trait objects because we need the flexibility of an open type. Shall use the date types in core. The API should be similar to the JPLEphemeris.jl. And the execution time of the |
a3a942c
to
bca9799
Compare
247d476
to
bd789c6
Compare
89c3b01
to
423d63b
Compare
Recommended by clippy
0b3b491
to
6f4a9d1
Compare
6f4a9d1
to
75b349c
Compare
Closing in favor of #15 |
Relates to #2
This is only a parser for the file record so far.
One design choice was to make all the vector types and the strings owned. The alternative would've been to tie the struct to a lifetime and avoid copying the 900 bytes of
prenul
,ftpstr
andpostnul
. But I decided this would be a minor gain against the downside of having to manage the extra lifetime.The test validates the values read against the values return by the NASA NAIF spy executable.
Another design choice was to keep the struct field names as close as possible to the ones in the spec, I am ambivalent about this.
Finally, another choice was to leave the
locfmt
field a string instead of transforming this into anenum
. The reason for this is because I kept this data structure relatively close to the binary format (for example by keepingprenul
,ftpstr
andpostnul
). I currently can't imagine a need for the user of thedaf/spk
library to care about these implementation details, so I will keep it this way. In the future, it will be hidden behind an API closer to the julia implementation anyway.Still to do:
initial_epoch
andfinal_epoch
to Date from j2000 Integrate ephemeris code with Date code #14