search

PoisotLab / GBIF.jl

Functions and types to access GBIF data from Julia
https://ecojulia.github.io/GBIF.jl/latest/
Other
19 stars 4 forks source link

Remove type piracy #43

Closed rafaqz closed 3 years ago

rafaqz commented 3 years ago

Adding convert(::AbstractString, ::Nothing) is type piracy on core julia types. It affects every package loaded in a session and all of Base Julia.. The main issue being that it will allow code to work on a system with GBIF.jl loaded that will fail on another system without GBIF loaded, which is not the worst kind of piracy, but we still can't.

We should really find where nothing is being returned, and wrap it with check that inserts missing instead. Is there an example taxon that triggers the bug?

codecov[bot] commented 3 years ago

Codecov Report

Merging #43 (8b01aed) into main (5385cc9) will decrease coverage by 0.07%. The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
- Coverage   85.95%   85.87%   -0.08%     
==========================================
  Files          10        9       -1     
  Lines         178      177       -1     
==========================================
- Hits          153      152       -1     
  Misses         25       25
Flag Coverage Δ
unittests 85.87% <81.81%> (-0.08%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/tables.jl 66.66% <66.66%> (ø)
src/types/GBIFRecords.jl 90.62% <87.50%> (-1.05%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5385cc9...8b01aed. Read the comment docs.

tpoisot commented 3 years ago

Let's re-open this when the Tables interface is fixed, as the PR currently mixes the two. There's a reason why this line is here, I'll dig it up in the meantime.

mkborregaard commented 3 years ago

Maybe just refactor out the Tables part and reopen? The type piracy part seems important

rafaqz commented 3 years ago

I must have rebased that tables code in by accident. Let me know next time before closing, as it wasn't possible to reopen this after a force-pushing the fix to this branch.

I made a new PR in #48