search

NOAA-EDAB / comlandr

https://noaa-edab.github.io/comlandr/
https://noaa-edab.github.io/comlandr/
Other
0 stars 0 forks source link

Sean overhaul #19

Closed slucey closed 2 years ago

slucey commented 2 years ago

Here's my overhaul of comlandr. We may need to resolve a few merge conflicts.

andybeet commented 2 years ago

@slucey I can't get the package to build from your branch. For example the get_discard_data.r has library statements and a call to get_observer_data

slucey commented 2 years ago

OK...I must have missed some files.

slucey commented 2 years ago

@andybeet see if that works now

andybeet commented 2 years ago

@slucey Almost. assign_area.R and calc_DK.R are inheriting parameter values from strat_prep which is a function survdat. Do we just need to hard code the @param declarations?

slucey commented 2 years ago

That will be something that I will fix. I copied some stuff over from survdat and obviously didn't change the @param designations.

andybeet commented 2 years ago

a <- comlandr::comland(channel, use.existing = "n",endyear=1970)

Pulling landings data from database. This could take a while (> 1 hour) ... 

 Error in if (substr(tables[itab], 1, 3) == "WOL") { : 
  missing value where TRUE/FALSE needed
slucey commented 2 years ago

@andybeet The comland function is deprecated. Use the get_comland_data function instead. After everything is merged I'll go through a remove unnecessary functions.

slucey commented 2 years ago

@andybeet I was just running a quick test on my end and realize that I don't have get_foreign on my branch. So you need to set that to F when running get_comland_data.

andybeet commented 2 years ago

ah ok. a <- comlandr::get_comland_data(channel,useForeign = F)

Error in if (any(filterByYear < 1964)) stop("Landings data start in 1964") : 
  missing value where TRUE/FALSE needed

I think maybe the default arguments for filterByYear, filterByArea, (maybe others) should either have a default value, or the code should deal with NA's. Maybe the former is better?

slucey commented 2 years ago

The default value is NA so you don't need to specify a range. I'll look into why that error is still occurring.

andybeet commented 2 years ago

So using useForeign = F i can run the code. All seems ok. You are right that there are a couple of warning messages relating to NAs being introduced by coercion. I need to figure out the get_foregin_data function to be able to take the arguments filterByYear, and filterByArea. Will add as an issue