OHI-Science / bali

Independent Ocean Health Index Assessment for Bali
1 stars 1 forks source link

Reviewing file and code uploads for meeting August 22 #3

Open jules32 opened 6 years ago

jules32 commented 6 years ago

Hello @twibawa,

To look through your code, I began by trying to run calculate_scores.R. This lets me incrementally fix any errors that occur. I will save each as a separate commit message so it's easier for you to review. I've pushed all my changes but haven't been able to complete it all. I will continue to review after our meeting. If you pull the bali repo, you will have all of my additions.

1. two placeholder data layers

I created 2 placeholder data layers and saved them to the /layers folder and layers.csv. Here is the commit

They are:

I did this because these two layers were registered but did not have associated .csv files, eg:

layers = ohicore::Layers('layers.csv', 'layers')
Error in file(file, "rt") : cannot open the connection
In addition: Warning message:
In file(file, "rt") :
  cannot open file 'layers/fp_fishing_gear_bali2016.csv': No such file or directory

(There was also a single typo in functions.R that I fixed)

2. CS function

calculate_scores.R failed at the CS function. I walked through the CA goal model and found a few things, listed here in the commit.

There were a few warning messages that I've highlighted in the code, and I suggested fixes for the error messages.

3. CP function

Next, the CP function failed. I ran out of time to fix this but this is as far as I got: We can walk through it together and I can show you how I troubleshoot if that is useful for you.

The extent variable returned NULL even though all three extent data layers exist in the /layers folder...I checked one by one:

## Julie testing
# layers$data$cs_mangrove_extent
# layers$data$cs_seagrass_extent
# layers$data$cp_coral_extent # NULL...but the data layer exists. Add it to layers.csv

It seems that coral wasn't registered in layers.csv. I added extent and health (commit here) and checked scenario_data_years and you've already updated that.

I've pushed this; we can continue during the call.