ptiede / VIDA.jl

EHT Image domain analysis through template matching.
MIT License
13 stars 4 forks source link

Upgrade HDF5 and `import DataFrames: stack` to avoid a clash? #52

Closed mcabbott closed 2 years ago

mcabbott commented 2 years ago

I don't know anything about this package but tests now pass locally.

The reason for suggesting this change is that Base exports stack from 1.9, creating a clash:

WARNING: both VIDA and Base export "stack"; uses of it in module Test_templates must be qualified

In fact DataFrames also exports stack, so there may already be a clash if those are used together. Recent versions of DataFrames extend the function from Compat / Base to avoid this, and this package depends on DataFrames, so an obvious solution would be to import and extend that: Then this package needs no new deps.

But latest DataFrames wants Compat 4, and this wants HDF5 0.15, which wants Compat 3. So this PR proposes to, first, allow HDF5 v0.16. I don't know what the changed or whether it matters: https://github.com/JuliaIO/HDF5.jl/releases/tag/v0.16.0

(jl_Xpc1Gh) pkg> st
Status `/private/var/folders/yq/4p2zwd614y59gszh7y9ypyhh0000gn/T/jl_Xpc1Gh/Project.toml`
  [34da2185] Compat v4.3.0
  [a93c6f00] DataFrames v1.4.1
  [f67ccb44] HDF5 v0.16.12
  [4096cdfb] VIDA v0.10.8 `https://github.com/mcabbott/VIDA.jl#patch-1`

The method defined here is for a type this package owns, so this is not piracy:

julia> @which VIDA.stack
VIDA

julia> methods(VIDA.stack)
# 1 method for generic function "stack" from VIDA:
 [1] stack(θ::T, θ1...) where T<:VIDA.AbstractTemplate
     @ ~/.julia/packages/VIDA/B7joP/src/templates/composite.jl:58

julia> @which VIDA.AbstractTemplate
VIDA

https://github.com/ptiede/VIDA.jl/blob/e161a11b07b1cac5272f708b1f663fd582a34e99/src/templates/composite.jl#L58-L60

And looking at the other methods, there is no reason to expect ambiguities:

julia> methods(stack)
# 6 methods for generic function "stack" from Base:
 [1] stack(df::AbstractDataFrame)
     @ DataFrames ~/.julia/packages/DataFrames/Lrd7K/src/abstractdataframe/reshape.jl:136
 [2] stack(df::AbstractDataFrame, measure_vars)
     @ DataFrames ~/.julia/packages/DataFrames/Lrd7K/src/abstractdataframe/reshape.jl:136
 [3] stack(df::AbstractDataFrame, measure_vars, id_vars; variable_name, value_name, view, variable_eltype)
     @ DataFrames ~/.julia/packages/DataFrames/Lrd7K/src/abstractdataframe/reshape.jl:136
 [4] stack(iter; dims)
     @ abstractarray.jl:2719
 [5] stack(f, iter; dims)
     @ abstractarray.jl:2748
 [6] stack(f, xs, yzs...; dims)
     @ abstractarray.jl:2749

Also, the earliest DataFrames version listed does already contain (and export) this function.

codecov[bot] commented 2 years ago

Codecov Report

Base: 93.14% // Head: 91.61% // Decreases project coverage by -1.52% :warning:

Coverage data is based on head (bfd2e71) compared to base (e5cb9c2). Patch has no changes to coverable lines.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #52 +/- ## ========================================== - Coverage 93.14% 91.61% -1.53% ========================================== Files 13 13 Lines 1138 1157 +19 ========================================== Hits 1060 1060 - Misses 78 97 +19 ``` | [Impacted Files](https://codecov.io/gh/ptiede/VIDA.jl/pull/52?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Paul+Tiede) | Coverage Δ | | |---|---|---| | [src/VIDA.jl](https://codecov.io/gh/ptiede/VIDA.jl/pull/52/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Paul+Tiede#diff-c3JjL1ZJREEuamw=) | `100.00% <ø> (ø)` | | | [src/io.jl](https://codecov.io/gh/ptiede/VIDA.jl/pull/52/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Paul+Tiede#diff-c3JjL2lvLmps) | `65.25% <0.00%> (-12.53%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Paul+Tiede). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Paul+Tiede)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

ptiede commented 2 years ago

Ya thank you for this! I'll add this and update the package. The package is going to undergo an overhaul soonish but at least things will work on 1.9 now.

mcabbott commented 2 years ago

Great!

Would you mind tagging a release? This will remove it from the list of problems seen by PkgEval when scanning the ecosystem for what 1.9 breaks.

ptiede commented 2 years ago

Done!