Add quality code tests: Aqua.jl + method ambiguities test (#921)

* Resolve method ambiguities * remove file * Update package_sanity_tests.jl * Update openstreetmap.jl * Update package_sanity_tests.jl * Update Project.toml * Update Project.toml * Update package_sanity_tests.jl * add Aqua badge * Update space_interaction_API.jl
JuliaDynamics · Nov 14, 2023 · de82efd · de82efd
1 parent fa03585
commit de82efd
Show file tree

Hide file tree

Showing 11 changed files with 38 additions and 21 deletions.
diff --git a/README.md b/README.md
@@ -4,6 +4,7 @@
 [![](https://img.shields.io/badge/DOI-10.1177/00375497211068820-purple)](https://journals.sagepub.com/doi/10.1177/00375497211068820)
 [![CI](https://github.com/JuliaDynamics/Agents.jl/workflows/CI/badge.svg)](https://github.com/JuliaDynamics/Agents.jl/actions?query=workflow%3ACI)
 [![codecov](https://codecov.io/gh/JuliaDynamics/Agents.jl/branch/main/graph/badge.svg)](https://codecov.io/gh/JuliaDynamics/Agents.jl)
+[![Aqua QA](https://raw.githubusercontent.com/JuliaTesting/Aqua.jl/master/badge.svg)](https://github.com/JuliaTesting/Aqua.jl)
 [![Package Downloads](https://shields.io/endpoint?url=https://pkgs.genieframework.com/api/v1/badge/Agents)](https://pkgs.genieframework.com?packages=Agents)
 
 Agents.jl is a pure [Julia](https://julialang.org/) framework for agent-based modeling (ABM): a computational simulation methodology where autonomous agents react to their environment (including other agents) given a predefined set of rules.
@@ -37,4 +38,4 @@ please cite the paper below:
   volume = {0},
   number = {0},
 }
-```
+```
diff --git a/docs/Project.toml b/docs/Project.toml
@@ -3,6 +3,7 @@ Agents = "46ada45e-f475-11e8-01d0-f70cc89e6671"
 BenchmarkTools = "6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf"
 BlackBoxOptim = "a134a8b2-14d6-55f6-9291-3336d3ab0209"
 CairoMakie = "13f3f980-e62b-5c42-98c6-ff1f3baf88f0"
+CellListMap = "69e1c6dd-3888-40e6-b3c8-31ac5f578864"
 DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
 DiffEqCallbacks = "459566f4-90b8-5000-8ac3-15dfb0a30def"
 Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
@@ -12,18 +13,17 @@ DrWatson = "634d3b9d-ee7a-5ddf-bec9-22491ea816e1"
 FileIO = "5789e2e9-d7fb-5bc7-8068-2c6fae9b9549"
 GLMakie = "e9467ef8-e4e7-5192-8a1a-b1aee30e663a"
 GraphRecipes = "bd48cda9-67a9-57be-86fa-5b3c104eda73"
-ImageMagick = "6218d12a-5da1-5696-b52f-db25d2ecc6d1"
 Graphs = "86223c79-3864-5bf0-83f7-82e725a168b6"
+ImageMagick = "6218d12a-5da1-5696-b52f-db25d2ecc6d1"
 LightOSM = "d1922b25-af4e-4ba3-84af-fe9bea896051"
-OSMMakie = "76b6901f-8821-46bb-9129-841bc9cfe677"
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
 Literate = "98b081ad-f1c9-55d3-8b20-4c87d4299306"
 Measurements = "eff96d63-e80a-5855-80a2-b1b0885c5ab7"
+OSMMakie = "76b6901f-8821-46bb-9129-841bc9cfe677"
 OrdinaryDiffEq = "1dea7af3-3e70-54e6-95c3-0bf5283fa5ed"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
 RecipesBase = "3cdcf5f2-1ef4-517c-9805-6587b60abb01"
 SimpleWeightedGraphs = "47aef6b3-ad0c-573a-a1e2-d07658019622"
 SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
-StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
 StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
-CellListMap = "69e1c6dd-3888-40e6-b3c8-31ac5f578864"
+StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
diff --git a/docs/src/devdocs.md b/docs/src/devdocs.md
@@ -19,7 +19,7 @@ In principle, the following should be done:
 1. Implement the `struct` that represents your new space, while making it a subtype of `AbstractSpace`.
 1. Extend `random_position(model)`.
 1. Extend `add_agent_to_space!(agent, model), remove_agent_from_space!(agent, model)`. This already provides access to `add_agent!, kill_agent!` and `move_agent!`.
-1. Extend `nearby_ids(position, model, r)`.
+1. Extend `nearby_ids(pos, model, r)`.
 1. Create a new "minimal" agent type to be used with [`@agent`](@ref) (see the source code of [`GraphAgent`](@ref) for an example).
 
 And that's it! Every function of the main API will now work. In some situations you might want to explicitly extend other functions such as `move_agent!` for performance reasons.

diff --git a/src/core/space_interaction_API.jl b/src/core/space_interaction_API.jl
@@ -56,20 +56,20 @@ remove_agent_from_space!(agent, model) = notimplemented(model)
 # %% IMPLEMENT: Neighbors and stuff
 #######################################################################################
 """
-    nearby_ids(position, model::ABM, r = 1; kwargs...) → ids
+    nearby_ids(pos, model::ABM, r = 1; kwargs...) → ids
 
 Return an iterable over the IDs of the agents within distance `r` (inclusive) from the given
-`position`. The `position` must match type with the spatial structure of the `model`.
+position. The position must match type with the spatial structure of the `model`.
 The specification of what "distance" means depends on the space, hence it is explained
 in each space's documentation string. Keyword arguments are space-specific and also
 described in each space's documentation string.
 
-`nearby_ids` always includes IDs with 0 distance to `position`.
+`nearby_ids` always includes IDs with 0 distance to `pos`.
 """
-nearby_ids(position, model, r = 1) = notimplemented(model)
+nearby_ids(pos, model, r = 1) = notimplemented(model)
 
 """
-    nearby_positions(position, model::ABM{<:DiscreteSpace}, r=1; kwargs...)
+    nearby_positions(pos, model::ABM{<:DiscreteSpace}, r=1; kwargs...)
 
 Return an iterable of all positions within "radius" `r` of the given `position`
 (which excludes given `position`).
@@ -85,7 +85,7 @@ For [`OpenStreetMapSpace`](@ref) this means "nearby intersections" and operates
 on the underlying graph of the OSM, providing the intersection nodes nearest to the
 given position.
 """
-nearby_positions(position, model, r = 1) = notimplemented(model)
+nearby_positions(pos, model, r = 1) = notimplemented(model)
 
 #######################################################################################
 # %% OPTIONAL IMPLEMENT
@@ -368,8 +368,8 @@ function random_nearby_agent(a, model, r = 1, f = nothing, alloc = false; kwargs
 end
 
 """
-    random_nearby_position(position, model::ABM, r=1, f = nothing, alloc = false; kwargs...) → position
-Return a random position near the given `position`.
+    random_nearby_position(pos, model::ABM, r=1, f = nothing, alloc = false; kwargs...) → position
+Return a random position near the given position.
 Return `nothing` if the space doesn't allow for nearby positions.
 
 The value of the argument `r` and possible keywords operate identically to [`nearby_positions`](@ref).

diff --git a/src/spaces/discrete.jl b/src/spaces/discrete.jl
@@ -75,10 +75,10 @@ function empty_positions(model::ABM{<:DiscreteSpace})
 end
 
 """
-    isempty(position, model::ABM{<:DiscreteSpace})
+    isempty(pos, model::ABM{<:DiscreteSpace})
 Return `true` if there are no agents in `position`.
 """
-Base.isempty(pos, model::ABM) = isempty(ids_in_position(pos, model))
+Base.isempty(pos::ValidPos, model::ABM{<:DiscreteSpace}) = isempty(ids_in_position(pos, model))
 
 """
     has_empty_positions(model::ABM{<:DiscreteSpace})

diff --git a/src/spaces/grid_single.jl b/src/spaces/grid_single.jl
@@ -57,7 +57,7 @@ end
 # move_agent! does not need be implemented.
 # The generic version at core/space_interaction_API.jl covers it.
 # `random_empty` comes from spaces/discrete.jl as long as we extend:
-Base.isempty(pos, model::ABM{<:GridSpaceSingle}) = abmspace(model).stored_ids[pos...] == 0
+Base.isempty(pos::ValidPos, model::ABM{<:GridSpaceSingle}) = abmspace(model).stored_ids[pos...] == 0
 # And we also need to extend the iterator of empty positions
 function empty_positions(model::ABM{<:GridSpaceSingle})
     Iterators.filter(i -> abmspace(model).stored_ids[i...] == 0, positions(model))

diff --git a/src/spaces/nothing.jl b/src/spaces/nothing.jl
@@ -19,6 +19,6 @@ function add_agent!(A::Type{<:AbstractAgent}, model::ABM{Nothing}, args::Vararg{
     add_agent_pos!(newagent, model)
 end
 
-nearby_ids(position, model::ABM{Nothing}, r = 1) = allids(model)
+nearby_ids(agent::AbstractAgent, model::ABM{Nothing}, r = 1) = allids(model)
 remove_agent_from_space!(agent, model::ABM{Nothing}) = nothing
 add_agent_to_space!(agent, model::ABM{Nothing}) = nothing
diff --git a/src/spaces/openstreetmap.jl b/src/spaces/openstreetmap.jl
@@ -963,8 +963,13 @@ ids_on_road(pos_1::Int, pos_2::Int, model::ABM{<:OpenStreetMapSpace}) =
         reverse_ids_on_road(pos_1, pos_2, model),
     ))
 
-Agents.nearby_positions(pos::Tuple{Int,Int,Float64}, model, args::Vararg{Any, N}; kwargs...) where {N} =
+function Agents.nearby_positions(
+        pos::Tuple{Int,Int,Float64}, 
+        model::ABM{<:OpenStreetMapSpace}, 
+        args::Vararg{Any, N}; kwargs...
+    ) where {N}
     nearby_positions(pos[1], model, args...; kwargs...)
+end
 
 function Agents.nearby_positions(
         position::Int,

diff --git a/test/Project.toml b/test/Project.toml
@@ -1,18 +1,22 @@
 [deps]
+Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
 Arrow = "69666777-d1a9-59fb-9406-91d4454c9d45"
 BenchmarkTools = "6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf"
-ColorTypes = "3da002f7-5984-5a60-b8a6-cbb66c0b333f"
 CSV = "336ed68f-0bac-5ca0-87d4-7b16caf5d00b"
+CairoMakie = "13f3f980-e62b-5c42-98c6-ff1f3baf88f0"
+ColorTypes = "3da002f7-5984-5a60-b8a6-cbb66c0b333f"
 Distributed = "8ba89e20-285c-5b6f-9357-94700520ee1b"
 GraphMakie = "1ecd5474-83a3-4783-bb4f-06765db800d2"
 Graphs = "86223c79-3864-5bf0-83f7-82e725a168b6"
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
 Makie = "ee78f7c6-11fb-53f2-987a-cfe4a2b5a57a"
-CairoMakie = "13f3f980-e62b-5c42-98c6-ff1f3baf88f0"
 OSMMakie = "76b6901f-8821-46bb-9129-841bc9cfe677"
 Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
 StableRNGs = "860ef19b-820b-49d6-a774-d7a799459cd3"
 Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
 StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+
+[compat]
+Aqua = "0.7"
diff --git a/test/package_sanity_tests.jl b/test/package_sanity_tests.jl
@@ -0,0 +1,6 @@
+using Aqua
+
+@testset "Code quality" begin
+	Aqua.test_all(Agents, ambiguities = false, unbound_args = false)
+	@test Test.detect_ambiguities(Agents) == Tuple{Method, Method}[]
+end
diff --git a/test/runtests.jl b/test/runtests.jl
@@ -150,6 +150,7 @@ function flocking_model_agent_step!(bird, model)
 end
 
 @testset "Agents.jl Tests" begin
+    include("package_sanity_tests.jl")
     include("model_creation_tests.jl")
     include("api_tests.jl")
     include("randomness_tests.jl")