Skip to content
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

Test with Aqua.jl #62

Merged
merged 15 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,20 @@ Artifacts = "1"
AutoHashEquals = "0.2"
Clustering = "0.15"
DataFrames = "1.5"
Dates = "1"
DelimitedFiles = "1"
Downloads = "1"
HORIZONS = "0.4"
HTTP = "1.9.5"
Healpix = "4"
InteractiveUtils = "1"
JLD2 = "0.4"
JSON = "0.21"
LazyArtifacts = "1"
LinearAlgebra = "1"
LsqFit = "0.15"
PlanetaryEphemeris = "0.8"
Printf = "1"
Quadmath = "0.5"
Roots = "2"
SPICE = "0.2"
Expand Down
2 changes: 1 addition & 1 deletion src/NEOs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export pv2kep, yarkp2adot
# Too Short Arc
export tooshortarc
# Gauss method
export gauss_method, gaussinitcond, gauss_refinement
export gauss_method, gaussinitcond
# Outlier rejection
export outlier_rejection
# Orbit determination
Expand Down
2 changes: 1 addition & 1 deletion src/observations/process_radec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function evaluate(res::AbstractVector{OpticalResidual{T, TaylorN{T}}}, x::Vector
end
return res_new
end
(res::AbstractVector{OpticalResidual{T, TaylorN{T}}})(x::Vector{T}) where {T <: Real} = evaluate(res, x)
(res::Vector{OpticalResidual{T, TaylorN{T}}})(x::Vector{T}) where {T <: Real} = evaluate(res, x)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naive question: what is the motivation for this change? (I think AbstractVector would allow applying this function to more types, e.g. SubArray{T,1} or SVector).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may perhaps not be relevant, but I thought it was easier to ask...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helps avoid an ambiguity with callable methods from Interpolations.jl which also dispatch on <:AbstractArray{T,N}; I agree it's perhaps a bit restrictive, and a more generic solution could be to define a OpticalResidualVector struct and define the callable methods over that struct. I would suggest to go with this approach for now, and revisit this if needed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yet another possibility is to avoid using Vector{T} for the calling argument type (again, maybe wrapping it somehow in another struct)


# Print method for OpticalResidual
# Examples:
Expand Down
19 changes: 0 additions & 19 deletions src/observations/topocentric.jl
Original file line number Diff line number Diff line change
Expand Up @@ -316,22 +316,3 @@ end
function obsposvelECI(x::RadarJPL{T}; eop::Union{EopIau1980, EopIau2000A} = eop_IAU2000A) where {T <: AbstractFloat}
return obsposvelECI(x.rcvr, datetime2et(x.date); eop)
end

# SatelliteToolboxTransformations.ecef_to_geocentric specialization for Vector{TaylorN{T}}
# See https://github.com/JuliaSpace/SatelliteToolboxTransformations.jl/blob/b7790389407a207499d343aa6d546ef54990baa2/src/reference_frames/geodetic_geocentric.jl#L31-L60
function SatelliteToolboxTransformations.ecef_to_geocentric(r_e::Vector{TaylorN{T}}) where {T<:AbstractFloat}

# Auxiliary variables.
x = r_e[1]
y = r_e[2]
z = r_e[3]
x² = x^2
y² = y^2
z² = z^2

lat = atan(z, √(x² + y²))
lon = atan(y, x)
r = √(x² + y² + z²)

return lat, lon, r
end
4 changes: 4 additions & 0 deletions test/Project.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[deps]
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
Expand All @@ -13,3 +14,6 @@ Query = "1a8c2f83-1ff3-5112-b086-8aa67b057ba1"
Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
TaylorIntegration = "92b13dbe-c966-51a2-8445-caca9f8a7d42"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[compat]
Aqua = "0.8"
33 changes: 33 additions & 0 deletions test/aqua.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using Test
using NEOs
using Aqua

@testset "Aqua tests (performance)" begin
# This tests that we don't accidentally run into
# https://github.com/JuliaLang/julia/issues/29393
# Aqua.test_unbound_args(NEOs)
ua = Aqua.detect_unbound_args_recursively(NEOs)
@test length(ua) == 0

# See: https://github.com/SciML/OrdinaryDiffEq.jl/issues/1750
# Test that we're not introducing method ambiguities across deps
ambs = Aqua.detect_ambiguities(NEOs; recursive = true)
pkg_match(pkgname, pkdir::Nothing) = false
pkg_match(pkgname, pkdir::AbstractString) = occursin(pkgname, pkdir)
filter!(x -> pkg_match("NEOs", pkgdir(last(x).module)), ambs)
for method_ambiguity in ambs
@show method_ambiguity
end
if VERSION < v"1.10.0-DEV"
@test length(ambs) == 0
end
end

@testset "Aqua tests (additional)" begin
Aqua.test_ambiguities(NEOs)
Aqua.test_all(
NEOs;
ambiguities=false,
piracies=(broken=true,)
)
end
1 change: 1 addition & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ testfiles = (
"propagation.jl",
"orbit_determination.jl",
"dataframes.jl",
"aqua.jl",
)

for file in testfiles
Expand Down
Loading