JuliaSpace / SatelliteToolbox.jl

A toolbox for satellite analysis written in julia language.
MIT License
249 stars 33 forks source link

Fix DatetoJD(::DateTime) on 32-bit platforms #43

Closed ThatcherC closed 4 years ago

ThatcherC commented 4 years ago

This PR fixes an error where calling DatetoJD on a Julia DateTime object would fail on a 32-bit system (a Raspberry Pi in my case). The error had to do with the use of the Int type in some function definitions in src/time/julian_day.jl and src/time/time.jl, which resolves to Int32 on a 32-bit machine. DateTime.year(), on the other hand, is an Int64, which causes a type error. My fix changes the function definitions in question to use the more permissive Integer type.

I also added a unit test that addresses the bug I fixed!

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 241


Totals Coverage Status
Change from base Build 240: 0.03%
Covered Lines: 1813
Relevant Lines: 2872

💛 - Coveralls
ronisbr commented 4 years ago

Hi @ThatcherC!

This is awesome, thank you very much! I'll merge as soon as the tests are completed.

It is nice to see someone using this package in a Raspberry Pi :) I would have thought that it would not be able to compile it. Is it possible, just for curiosity, to tell me which model are you using?

ThatcherC commented 4 years ago

Hey @ronisbr - thanks! Glad to have contributed to the project- it's an amazing resource. I was using SatelliteToolbox.jl on a Raspberry Pi 3B+. Julia is slow to start on that platform, and all programs run slowly at first, though I think this is all due to Julia's JIT compiler. Once it's up and running though, Julia+SatelliteToolbox work just fine on the Pi.

I don't know if the Pi is a platform you're interested in targeting, but it could be useful to add a 32-bit Debian option to the test running you have set up on this repo. For now though, it all seems to work perfectly well with this new patch.