search

afsc-gap-products / akgfmaps

Make AFSC bottom trawl survey maps and retrieve map layers
Other
17 stars 6 forks source link

inpfc #108

Closed BenWilliams-NOAA closed 9 months ago

BenWilliams-NOAA commented 9 months ago

https://github.com/afsc-gap-products/akgfmaps/blob/04a47bb3c39933bd9c9ecd89d59ad3ffb11761ed/R/get_base_layers.R#L70

looks like the inpfc.goa and inpfc.ai base layers are incomplete.

sean-rohan-NOAA commented 9 months ago

@BenWilliams-NOAA Sorry, I'm not entirely sure what you mean. I'm wondering if you mean:

  1. get_base_layers(select.region = "inpfc.goa") and get_base_layers(select.region = "inpfc.ai") only return the INPFC region.
  2. The shapefiles themselves are incorrect.

If it's the first one, that was intentional. However, you're the second person to mention that and I'd be open to alternatives. For example, automatically including INPFC strata with regional outputs (e.g. get_base_layers(select.region = "ai") could include a inpfc.stratum sf object for the region), returning a subset of layers, or only having the get_inpfc_strata() function return the INPFC shapefiles. I'm not sure what would make the most sense to users.

BenWilliams-NOAA commented 9 months ago

Hey @sean-rohan-NOAA, yeah the 1st one. Was just exploring the package and when I went to plot the inpfc.goa I found that it is missing a lot of the inputs found when using just goa. Wasn't sure on why that was the case. Not knowing how/who all are using this package, you could simply remove them, or add a description of how to incorporate them into a figure. Nice package by the way!

sean-rohan-NOAA commented 9 months ago

Thanks! And thank you for bringing this up.

Definitely something to consider. Based on the feedback so far (n=2), I'm gravitating towards just including the INPFC strata in the regional outputs instead of allowing "inpfc.goa"/"inpfc.ai" for get_base_layers(select.region). I suppose that would be more consistent with region naming anyhow.

sean-rohan-NOAA commented 9 months ago

After some consideration, it seems like the behavior of get_base_layers() didn't seem to make sense since it was inconsistent with other regions. As of version 3.4.1, the function returns an INPFC stratum layer (inpfc.stratum) when select.region is 'ai', 'ai.west', 'ai.central', 'ai.east', 'goa', 'goa.west', or 'goa.east'. It no longer accepts 'inpfc.goa' or 'inpfc.ai' as arguments to 'select.region'.

@BenWilliams-NOAA Thanks again for bringing this up.

Closed by #109