Chemellia / ChemistryFeaturization.jl

Interface package for featurizing atomic structures
https://chemistryfeaturization.chemellia.org/dev/
MIT License
41 stars 14 forks source link

Fix doctests #126

Closed thazhemadam closed 2 years ago

thazhemadam commented 2 years ago
julia> WS2_v = AtomGraph(joinpath("docs/src", "files", "mp-224.cif"), use_voronoi=true)

doesn't seem to work anymore, as it returns a missing instead of an AtomGraph.

The following error gets thrown -

BoundsError(([0.3716777477498808 0.0 0.0 0.9704476083038442 0.0 0.9704476083038442; 0.0 0.3716777477498808
0.9704476083038442 0.0 0.9704476083038442 0.0; 0.0 0.9704476083038442 1.0 0.0 0.02318554171415271
0.31893970588360054; 0.9704476083038442 0.0 0.0 1.0 0.31893970588360054 0.02318554171415271; 0.0
0.9704476083038442 0.02318554171415271 0.31893970588360054 1.0 0.0; 0.9704476083038442 0.0
0.31893970588360054 0.02318554171415271 0.0 1.0], ["W", "W", "S", "S", "S", "S"]), 3)
codecov[bot] commented 2 years ago

Codecov Report

Merging #126 (8b602d5) into main (0dd3b61) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #126   +/-   ##
=======================================
  Coverage   86.19%   86.19%           
=======================================
  Files          18       18           
  Lines         507      507           
=======================================
  Hits          437      437           
  Misses         70       70           
Impacted Files Coverage Δ
src/utils/graph_building.jl 98.36% <100.00%> (ø)

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 0dd3b61...8b602d5. Read the comment docs.

rkurchin commented 2 years ago

Okay, should we just merge this? I think at least fewer of them are failing than before... 🙄 though I'm sort of inclined to just ditch doctests given how finicky they are

thazhemadam commented 2 years ago

I think we should merge this for now. We can get back to the doctests later, but this also fixes up the problem of the docs not building at all IIRC. Either way, this has been around for a long time now and should probably be merged right away.