BiologicalRecordsCentre / sparta

Species Presence/Absence R Trends Analyses
http://biologicalrecordscentre.github.io/sparta/index.html
MIT License
21 stars 24 forks source link

Added new detection_diagnosis function #180

Closed drnickisaac closed 4 years ago

drnickisaac commented 4 years ago

I've added a new function. for some reason it's not added to the NAMESPACE on my machine. But it should work ....

AugustT commented 4 years ago

Without the function in the namespace users will not be able to use it so that needs to be solved before we merge it in. Try removing the blank line on L18

codecov-io commented 4 years ago

Codecov Report

Merging #180 into master will decrease coverage by 1.07%. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
- Coverage   59.79%   58.71%   -1.08%     
==========================================
  Files          82       83       +1     
  Lines        2858     2638     -220     
==========================================
- Hits         1709     1549     -160     
+ Misses       1149     1089      -60
Impacted Files Coverage Δ
R/detection_phenology.R 85.29% <ø> (+1.96%) :arrow_up:
R/detection_diagnosis.R 0% <0%> (ø)
R/cat_breaks.R 80% <0%> (-6.85%) :arrow_down:
R/read_frescalo.R 65.38% <0%> (-6.5%) :arrow_down:
R/plot_Tfactor.R 40.19% <0%> (-6.48%) :arrow_down:
R/all_oneplots.R 78.78% <0%> (-4.36%) :arrow_down:
R/plot_GIS.R 65.51% <0%> (-4.35%) :arrow_down:
R/z_value.R 72.72% <0%> (-4.2%) :arrow_down:
R/plotUK_gr.R 80% <0%> (-4%) :arrow_down:
R/plotUK_gr_cats.R 90.9% <0%> (-3.21%) :arrow_down:
... and 27 more

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 4f927cd...f43a37b. Read the comment docs.

drnickisaac commented 4 years ago

Ok, I have fixed this on my local branch and pushed to Github. I think this will automatically update my Merge Pull Request

AugustT commented 4 years ago

@drnickisaac Am I right in thinking this is only a plotting function? It does not look like it returns any statistics? If so I suggest the name is change to something starting plot_. You have also not created a test for this function. If you intend for it to only plot (i.e. no data returned), then a simple test that it runs without error would do for now. This is a little tricky because you need an OccDet object to test it on, happy to park this for now and create an issue that we need a test for it as we could add a test for plot.OccDet at the same time.

drnickisaac commented 4 years ago

I have renamed the new function as plot_DetectionOverTime. I've also renamed the detection_phenology() function as plot_DetectionPhenology