Ability to split / aggregate characteristics based on hydrofab refactor workflow.

Will document the idea later on, but we need to create a function to take attributes from one scale of catchments and split and / or aggregate them to another set of catchments.

haha we are working towards this too - we are testing out the stream cats data (see here: s3://nextgen-hydrofabric/streamcats/) with a zonal-esque approach that used data.table/collapse:
https://github.com/mikejohnson51/zonal/blob/18ef9f684bfdf32f732efa69747023526e504803/R/weights.R#L68

For PUMP-ML workflows we've aggregated characteristics to the NHM reference fabric. We haven't dealt with split catchments, but I'll start by describing our workflow for aggregation using just the tabular data.
library(tidyverse)
library(nhdplusTools)

# First, define a function to grab characteristics of interest. So far we've been downloading the data ourselves,
# but I'll use nhdplusTools helpers in the future now that I know about these capabilities to interface with NLDI.

#' @param varname character vector of desired variables retrieved from /link{discover_nldi_characteristics}
#' @param ids numeric vector of identifiers from the specified reference fabric
#' @param reference_fabric (not used) will be used to allow future specification of alternate reference fabrics
#' @importFrom dplyr bind_rows filter select
#' @importFrom tidyselect everything
get_catchment_characteristics <- function(varname, ids, reference_fabric = "nhdplusv2"){
  nldi_vars <- bind_rows(
    lapply(ids, function(x){
      nldi_ft <- list(featureSource = "comid", featureID = x)
      nldi_vars_ft <- bind_rows(get_nldi_characteristics(nldi_ft, type = "all"))
      nldi_vars_ft_subset <- filter(nldi_vars_ft, characteristic_id %in% varname)
      nldi_vars_ft_out <- select(mutate(nldi_vars_ft_subset,
                                        comid = as.numeric(x),
                                        characteristic_value = as.numeric(characteristic_value)),
                                 comid, everything())
      return(nldi_vars_ft_out)
    })
  )
  return(nldi_vars)
}

# Next, define a function to aggregate characteristics assuming we have a crosswalk of ids between the two fabrics
# and preferences from the user for which summary statistics to use when aggregating.

#' @param vars data frame containing two columns: "characteristic_id", which is a character vector of desired
#' variables retrieved from /link{discover_nldi_characteristics}; and "aggregation_statistic", which is a character
#' vector indicating the summary statistic that should be applied to each variable. Acceptable values for
#' "aggregation_statistic" include "sum", "length_weighted_mean", "area_weighted_mean", "min", and "max".
#' @param ids_xwalk data frame containing two columns: "id", which is a numeric vector of identifiers at the
#' desired scale and "comid" which is a numeric vector of NHDPlusv2 identifiers.
#' @importFrom sf st_drop_geometry
#' @importFrom dplyr select left_join group_by summarize ungroup
#' @importFrom tidyr pivot_wider
aggregate_catchment_characteristics <- function(vars, ids_xwalk){

  # download nldi characteristics for the requested comids
  nhd_vars <- get_catchment_characteristics(varname = vars$characteristic_id,
                                            ids = ids_xwalk$comid)

  # download nhdplus subset so that we have areasqkm and lengthkm
  # cols (there's probably a slicker way to do this)
  nhd_lines <- sf::st_drop_geometry(get_nhdplus(comid = c(ids_xwalk$comid), realization = "flowline"))
  nhd_subset <- select(nhd_lines, comid, areasqkm, lengthkm)

  # format characteristics data frame
  nhd_subset_w_vars <- left_join(x = left_join(x = nhd_subset, y = nhd_vars, by = "comid"),
                                 y = ids_xwalk, by = "comid")
  nhd_subset_w_vars_wide <- pivot_wider(nhd_subset_w_vars, !percent_nodata,
                                        names_from = characteristic_id,
                                        values_from = characteristic_value)

  # assign columns based on the desired summary operation
  cols_sum <- vars$characteristic_id[vars$aggregation_statistic == "sum"]
  cols_area_wtd_mean <- vars$characteristic_id[vars$aggregation_statistic == "area_weighted_mean"]
  cols_length_wtd_mean <- vars$characteristic_id[vars$aggregation_statistic == "length_weighted_mean"]
  cols_min <- vars$characteristic_id[vars$aggregation_statistic == "min"]
  cols_max <- vars$characteristic_id[vars$aggregation_statistic == "max"]

  # aggregate NHDPlusv2 attributes to desired catchments
  nhd_vars_aggregated <- nhd_subset_w_vars_wide |>
    group_by(id) |>
    summarize(
      areasqkm_sum = sum(areasqkm),
      lengthkm_sum = sum(lengthkm),
      across(any_of(cols_area_wtd_mean), weighted.mean, w = areasqkm, na.rm = TRUE, .names = "{col}_area_wtd"),
      across(any_of(cols_length_wtd_mean), weighted.mean, w = lengthkm, na.rm = TRUE, .names = "{col}_length_wtd"),
      across(any_of(cols_sum), sum, na.rm = TRUE, .names = "{col}_sum"),
      across(any_of(cols_min), min, na.rm = TRUE, .names = "{col}_min"),
      across(any_of(cols_max), max, na.rm = TRUE, .names = "{col}_max")
    ) |>
    ungroup()

  return(nhd_vars_aggregated)
}

# Apply the functions above to an example.
# define the desired characteristics and their corresponding summary statistics
vars <- data.frame(characteristic_id = c("CAT_EWT","CAT_WILDFIRE_2011"),
                   aggregation_statistic = c("area_weighted_mean", "min"))

# we use a crosswalk that tells us how the ids from the desired fabric (e.g. NHM fabric)
# correspond to NHDPlusv2 ids. Include crosswalk for one segment as an example:
catchment_xwalk <- data.frame(id = rep(1637, 8),
                              comid = c(4146594, 4146596, 4147370, 4147376,
                                        4147378, 4147380, 4147382, 4147396))
> aggregate_catchment_characteristics(vars, catchment_xwalk)
# A tibble: 1 x 5
     id areasqkm_sum lengthkm_sum CAT_EWT_area_wtd CAT_WILDFIRE_2011_min
  <dbl>        <dbl>        <dbl>            <dbl>                 <dbl>
1  1637         47.3         23.1            -23.3                     0
>

Is this roughly what you had in mind? The use case to split catchments is less obvious to me right now given the tabular data, I'll check out the zonal stats approach Mike linked to above.

This is exactly what I had in mind. The catchment splitting is going to require a little bit more in the main pipeline and we'll have to bring in a lookup table, but what you've got here is most of it and gives me a good sense for how this should work.

I've got a script (below) running for crosswalking nhdv2 catchment level variables to nhgf refactor. It aggregates and area-weights split catchments when it needs to, sum or mean. I tested with basin area and the numbers add up, even with nhgf divides that include split catchments.
Dave B graciously provided me with a refactor nhgf that includes a split_divides layer so we can get those split catchment areas. That was the missing piece.
So that's the good news, the bad news is that it is ridiculously slow. I'm doing everything in an admittedly tedious way just to get the concepts working.
Since I'm not a whiz at "doing things faster"...perhaps someone can have a look and see a way to optimize this? Like...a lot? It's so slow. And maybe test another variable or two?
library(sf)
library(beepr)
library(dplyr)
library(nhdplusTools)
library(sbtools)

#make a crosswalk from data compiled for nhdv2 catchments to nhgf
#works great! just unbearably slow.
#so i guess that makes it not great.
#
#11/1/2022 andrew sekellick
#updated 11/17/2022
#
#for nhdv2 to nhgf we have aggregated catchments and split catchments (or both, or just 1)
#
#this code works on the refactor nhgf and requires the version that includes the "split_divide" layer
#
#using nhdplusTools::get_nldi_characteristics() to get local basin area from the web is
#not much difference in speed as having a table on file. possibly even faster this way?
#i realize with the test variable being basin area i could have just used that but whatever, i'm trying stuff.
#this way you don't have to keep basin area on hand for other variables
#
#using catchment area as test variable and the final numbers look good.

workdir <- "C://nhgf"

#i'm using basin area as my test variable for crosswalking
#get it from here: https://www.sciencebase.gov/catalog/item/57976a0ce4b021cadec97890
#or use the snippet below to download and unzip
item_file_download('57976a0ce4b021cadec97890', destinations = file.path(workdir, 'basin_char.zip'), names = 'BASIN_CHAR_CAT_CONUS.zip')
unzip(file.path(workdir, 'basin_char.zip'), exdir = workdir)
file.remove(file.path(workdir, 'basin_char.zip'))

#test variable
bareavar <- read.csv(file.path(workdir, 'BASIN_CHAR_CAT_CONUS.txt')) %>%
  subset(select = c(COMID, CAT_BASIN_AREA))

#the nhgf refactor geopackage, stored locally as a dataframe.
#get it from here: https://www.sciencebase.gov/catalog/item/61fbfdced34e622189cb1b0a
#i was having a look at geometry, but I don't really need it.
divideswithgeom <- as.data.frame(st_read("C:\\nhgf\\refactor_02.gpkg", layer='refactored_divides'))
divides <- subset(divideswithgeom, select = -c(geom))

#this is the split divides layer
splitdivideswithgeom <- as.data.frame(st_read("C:\\nhgf\\refactor_02.gpkg", layer='split_divides'))
splitdivides <- subset(splitdivideswithgeom, select = -c(geom))

#choose sum or avg method (don't want to use sum for temperature).
#i haven't really tested avg
method <- "sum"
#method <- "avg"

starttime <- Sys.time()

#making an empty data frame to fill in later.
alldata <- data.frame()

#loop through all rows in the nhgf divides layer.
#you can do a test run with x number of rows.

for (row in 1:100)
  #for (row in 1:nrow(divides))
{
  print(row)
  #select the nhgf row
  tmpdivide <- divides[row,]
  #split the member_COMID column by commas to get list of COMIDs in the selected NHGF catchment
  tmpmembers <- unlist(strsplit(tmpdivide$member_COMID, ","))
  #making an empty dataframe
  tmptotal <- data.frame()
  #this loop checks each item in the list of COMIDs per nhgf divide
  for (membs in tmpmembers)
  {
    if (grepl(".", membs, fixed = TRUE) == TRUE)
    {
      #the if looks for a ".", which means it's a split catchment.
      #so we need to split to get that COMID value.
      testsplit <- unlist(strsplit(membs, ".", fixed = TRUE))
      testcomid <- testsplit[1]
      #this next chunk gets the total comid catchment area from nhdplustools online
      unsplit_area <-
        as.data.frame(get_nldi_characteristics(list(featureSource = "comid", featureID = testcomid), type = "local")) %>%
        filter(local.characteristic_id == "CAT_BASIN_AREA") %>%
        select(local.characteristic_value) %>%
        unlist()
      #this gets the area of the just the split catchment, thanks to Dave B for providing this.
      split_area <- filter(splitdivides, comid_part == membs)
      if (method == "avg")
      {
        newvalue <- (filter(bareavar, COMID == testcomid))
      } else if (method == "sum")
      {
        #this is the area weighting part!
        #it uses area of split_catchment divided by area of full nhd catchment, multiplied by the variable value
        newvalue <- (filter(bareavar, COMID == testcomid)) * (as.numeric(split_area) / as.numeric(unsplit_area))
      }
      #tmptotal is the total from all the member comids in that nhgf divide.
      #so it can be a couple of rows per divide
      tmptotal <- bind_rows(tmptotal, newvalue)
    }
    else
      #the else here is for unsplit catchments (no ".")
    {
      newvalue <- filter(bareavar, COMID == membs)
      tmptotal <- bind_rows(tmptotal, newvalue)
    }
  }
  #now we're summarizing all the pieces to get 1 value per nhgf catchment
  if (method == "avg")
  {
    tmptotalsum <- summarize_all(tmptotal, mean)
  } else if ( method == "sum")
  {
    tmptotalsum <- summarize_all(tmptotal, sum)
  }
  tmptotalsum <- subset(tmptotalsum, select = -c(COMID))
  tmpdivide <- as.data.frame(c(tmpdivide, tmptotalsum))
  #add the row that's the result of this loop to dataframe.
  alldata <- bind_rows(alldata, tmpdivide)
}

endtime <- Sys.time()

endtime - starttime

#beep
beep(2)

Here's a link to the refactor gpkg that includes the split_divide layer:
https://doimspp-my.sharepoint.com/:u:/g/personal/ajsekell_usgs_gov/Effo4XP4-fpOnktiz-Ay_ywBeR-B903AR3ag07N_HK2fXA?e=1lPLJX
(Dave I assume it's okay to share that)

OK, I took a look at the refactored hydrofabric and @ajsekell's code and came up with the following solution to rescale (i.e., split or aggregate) catchment characteristics:
# The code below calls a function that grabs the catchment characteristics. I haven't defined
# the function here, but it is included in my previous comment within this GitHub issue.

# Define a function to rescale the catchment attributes using the tabular data.
# This function is a combination of Andrew Sekellick's workflow to accommodate
# split catchments and a function I previously included in this GitHub issue
# to aggregate NHDPlusv2 attributes to new fabrics.

#' @param vars data frame containing two columns: "characteristic_id" is a character
#' vector of desired variables to retrieve from /link{discover_nldi_characteristics};
#' "summary_statistic", is a character vector indicating which summary statistic
#' should be applied to rescale each characteristic. Accepted values include
#' "sum," "length_weighted_mean," "area_weighted_mean," "min," and "max."
#' @param lookup_table data frame containing at least three columns: "ID" is a
#' numeric vector of identifiers at the desired scale; "comid" is a numeric vector
#' of NHDPlusv2 identifiers; "member_comid" contains the formatted NHDPlusv2 COMIDs,
#' for example, in the case of split catchments. If catchments have not been split,
#' the columns "comid" and "member_comid" should be identical.
#' @param refactored_areas data frame containing columns "FEATUREID" and "areasqkm."
#' Used to retrieve adjusted catchment areas in the case of split catchments.
#'
#' @importFrom dplyr left_join rename_with mutate across group_by summarize ungroup distinct
#' @importFrom tidyr pivot_wider
#' @importFrom tidyselect starts_with any_of
#'
rescale_nldi_characteristics <- function(vars, lookup_table, refactored_areas){

  # check that the inputs match what we are expecting
  if(!all(c("characteristic_id", "summary_statistic") %in% names(vars))){
    stop("Check that vars contains columns 'characteristic_id' and 'summary_statistic'")
  }
  summary_stat_acceptable <- vars$summary_statistic %in% c("min", "max", "sum",
                                                           "length_weighted_mean",
                                                           "area_weighted_mean")
  if(!all(summary_stat_acceptable)){
    stop("Check that all entries in vars$summary_statistic match accepted values")
  }
  if(!all(c("comid", "member_comid", "ID") %in% names(lookup_table))){
    stop("Check that lookup_table contains columns 'comid,' 'member_comid,' and 'ID'")
  }

  # omit any duplicated rows in the lookup table
  lookup_table <- distinct(lookup_table)

  # download nldi characteristics for the requested comids and pivot to wide format
  nhd_vars <- get_catchment_characteristics(varname = vars$characteristic_id,
                                            ids = unique(lookup_table$comid))
  nhd_vars_wide <- pivot_wider(nhd_vars, !percent_nodata,
                               names_from = characteristic_id,
                               values_from = characteristic_value)

  # get comid catchment areas, adjusting area for catchments that have been "split"
  nhd_subset <- get_catchment_areas(unique(lookup_table$member_comid), refactored_areas)

  # combine the nldi characteristics with the catchment identifier and basin area
  cat_vars_unscaled <- lookup_table |>
    left_join(y = nhd_subset, by = c("member_comid","comid")) |>
    left_join(y = nhd_vars_wide, by = "comid")

  # rescale the nldi characteristics if needed (i.e., for split catchments)
  if(!all(cat_vars_unscaled$comid == cat_vars_unscaled$member_comid)){
    var_names <- unique(nhd_vars$characteristic_id)
    cat_vars_scaled <- mutate(cat_vars_unscaled, across(any_of(var_names), ~.x*split_area_prop))
  } else {
    cat_vars_scaled <- cat_vars_unscaled
  }

  # assign columns based on the desired summary operation
  cols_sum <- vars$characteristic_id[vars$summary_statistic == "sum"]
  cols_area_wtd_mean <- vars$characteristic_id[vars$summary_statistic == "area_weighted_mean"]
  cols_length_wtd_mean <- vars$characteristic_id[vars$aggregation_statistic == "length_weighted_mean"]
  cols_min <- vars$characteristic_id[vars$summary_statistic == "min"]
  cols_max <- vars$characteristic_id[vars$summary_statistic == "max"]

  # rescale NHDPlusv2 attributes to desired catchments
  # !note that there are currently no adjustments made for the length of split flowlines
  nhd_vars_rescaled <- cat_vars_scaled |>
    group_by(ID) |>
    summarize(
      areasqkm_sum = sum(split_basin_areasqkm),
      lengthkm_sum = sum(lengthkm),
      across(any_of(cols_area_wtd_mean), weighted.mean, w = split_basin_areasqkm, na.rm = TRUE, .names = "{col}_area_wtd"),
      across(any_of(cols_length_wtd_mean), weighted.mean, w = lengthkm, na.rm = TRUE, .names = "{col}_length_wtd"),
      across(any_of(cols_sum), sum, na.rm = TRUE, .names = "{col}_sum"),
      across(any_of(cols_min), min, na.rm = TRUE, .names = "{col}_min"),
      across(any_of(cols_max), max, na.rm = TRUE, .names = "{col}_max")
    ) |>
    ungroup()

  return(nhd_vars_rescaled)
}

# Define the function `get_catchment_areas` that's called in `rescale_nldi_characteristics`

#' @title Get basin area and flowline length for NHDPlusV2 COMIDs
#'
#' @description
#' Get catchment area and flowline length for NHDPlusv2 COMID(s) of interest.
#' If any COMID represents a split catchment, the split catchment areas are
#' returned, along with a column that represents the proportion of the original
#' NHDPlusv2 catchment area that is covered by the split catchment.
#'
#' @param comids integer vector or character vector containing NHDPlusV2 identifiers
#' @param refactored_areas data frame containing columns "FEATUREID" and
#' "areasqkm." Used to retrieve adjusted catchment areas in the case of split
#' catchments.
#'
#' @importFrom sf st_drop_geometry
#' @importFrom dplyr mutate select right_join left_join filter rename bind_rows
#'
get_catchment_areas <- function(comids, refactored_areas){

  # format comids (omit suffix "." in the case of split catchments)
  comids_fmt <- data.frame(member_comid = comids) |>
    mutate(comid = gsub("\\..*", "", member_comid))

  # fetch basin area for all comids
  nhd_lines <- sf::st_drop_geometry(get_nhdplus(comid = c(unique(comids_fmt$comid))))
  nhd_subset <- nhd_lines |>
    mutate(comid = as.character(comid)) |>
    select(comid, areasqkm, lengthkm) |>
    right_join(x = comids_fmt, by = "comid")

  # handle "split" catchments (if applicable)
  if(all(comids_fmt$member_comid == comids_fmt$comid)){
    nhd_subset_out <- nhd_subset |>
      mutate(comid = as.integer(comid))
  } else {
    # create data frame containing the "split" catchments
    comids_to_split <- filter(nhd_subset, grepl(".", member_comid, fixed = TRUE))

    # get the basin area for the split catchments
    split_areas <- refactored_areas |>
      filter(FEATUREID %in% comids_to_split$member_comid) |>
      rename(split_basin_areasqkm = areasqkm)
    split_areas_fmt <- left_join(x = comids_to_split, y = split_areas, by = c("member_comid" = "FEATUREID"))

    # create data frame containing the unsplit catchments
    comids_no_split <- filter(nhd_subset, !grepl(".", member_comid, fixed = TRUE))
    unsplit_areas_fmt <- mutate(comids_no_split, split_basin_areasqkm = areasqkm)

    # bind split and unsplit catchments back together
    nhd_subset_out <- bind_rows(unsplit_areas_fmt, split_areas_fmt) |>
      mutate(comid = as.integer(comid),
             split_area_prop = split_basin_areasqkm/areasqkm)
  }

  return(nhd_subset_out)
}

# Apply the functions above to an example.

# First, define the attributes of interest and how they should be
# summarized when aggregated or split.
vars <- data.frame(characteristic_id = c("CAT_EWT","CAT_WILDFIRE_2011", "CAT_BASIN_AREA"),
                   summary_statistic = c("area_weighted_mean", "min","sum"))

# Next, get the lookup table from the refactored hydrofabric that
# contains information about how NHDPlusv2 catchments are split.
file_name <- "refactor_02.gpkg"
file_path <- file.path(tempdir(), file_name)
gpkg_file <- sbtools::item_file_download("61fbfdced34e622189cb1b0a",
                                         destinations = file_path,
                                         names = file_name,
                                         overwrite_file = TRUE)
lookup_table <- sf::st_read(gpkg_file, layer= "lookup_table") |>
  rename(comid = NHDPlusV2_COMID, member_comid = member_COMID, ID = reconciled_ID) |>
  # subset lookup table for this example
  filter(ID %in% c(10012268, 10012979, 10024047, 10024048, 10024049, 10024050))
lookup_table
> lookup_table
    comid member_comid       ID LevelPathID
1 4146596      4146596 10012268     2132667
2 4147382      4147382 10012268     2132667
3 4147378      4147378 10012979     2130154
4 4147370      4147370 10012979     2130154
5 4147396    4147396.1 10024047     2123206
6 4147396    4147396.2 10024048     2123206
7 4147380    4147380.1 10024049     2123206
8 4147380    4147380.2 10024050     2123206

# We will also need the dataset "split_divides" that gives the basin
# area for the split catchments.
split_divides <- sf::st_read(gpkg_file, layer = "split_divides")

# rescale the characteristics to the refactored hydrofabric
rescale_nldi_characteristics(vars, lookup_table, refactored_areas = split_divides)
># A tibble: 6 x 6
        ID areasqkm_sum lengthkm_sum CAT_EWT_area_wtd CAT_BASIN_AREA_sum CAT_WILDFIRE_2011_min
     <dbl>        <dbl>        <dbl>            <dbl>              <dbl>                 <dbl>
1 10012268        12.9          6.30           -25.5               12.9                      0
2 10012979         4.78         3.31           -11.9                4.77                     0
3 10024047         6.51         4.31           -24.6                6.52                     0
4 10024048         1.44         4.31            -5.44               1.44                     0
5 10024049         5.84         5.10           -10.8                5.84                     0
6 10024050         7.56         5.10           -14.0                7.56                     0
>

I downloaded the refactored hydrofabric from ScienceBase and tested this function on the same subset of 100 ID's that @ajsekell used, and got the ~same values for the summed CAT_BASIN_AREA (within rounding error; r2 = 1, slope = 1, intercept = 0). I'd appreciate any help with further testing though, especially for split catchments - @ajsekell or @mjcashman-usgs, maybe you have some examples in mind?
Re. run time, this still isn't super quick and rescaling 100 ID's takes ~3 minutes. We may be able to speed that up, but some initial profiling suggests most of the time is in downloading the NLDI characteristics.

Sounds good, thanks @dblodgett-usgs!

Awesome! That's almost a minute and a half faster than my script for 100 IDs.
I'll try to run yours with something like temperature to test a weighted average instead of sum but won't get to it until Monday at the earliest.
Sort of on topic, for aggregating up to a POI hydrofabric, we'd need to aggregate to the refactor fabric first (with this code) and then aggregate again to the POI fabric right? Don't think we could easily go directly from NHD to a POI fabric...

Cool -- progress.
I refactored your example a bit and think I have a good base implementation now @lekoenig
Note that I pulled out a stand alone function that runs the actual statistics and have all the data munging separate. I'll try and find some time to work this into nhdplusTools in the next few days, but this function layout should do the trick if you all want to try scaling this up.
library(tidyverse)
library(nhdplusTools)

get_charachteristics_metadata <- function() {

  u <- "https://prod-is-usgs-sb-prod-publish.s3.amazonaws.com/5669a79ee4b08895842a1d47/metadata_table.tsv"

  f <- file.path(nhdplusTools_data_dir(), "metadata_table.tsv")

  if(!file.exists(f)) resp <- httr::RETRY("GET", u, httr::write_disk(f))

  read.delim(f, sep = "\t")
}

#' @param varname character vector of desired variables
#' @param ids numeric vector of identifiers from the specified reference fabric
#' @param reference_fabric (not used) will be used to allow future specification of alternate reference fabrics
#' @importFrom dplyr bind_rows filter select
#' @importFrom tidyselect everything
get_catchment_characteristics <- function(varname, ids, reference_fabric = "nhdplusv2"){

  metadata <- get_charachteristics_metadata()

  bind_rows(
    lapply(varname, function(x) {
      i <- metadata[metadata$ID == x,]

      id <- gsub("https://www.sciencebase.gov/catalog/item/", "", i$datasetURL)

      bucket <- arrow::s3_bucket("s3://prod-is-usgs-sb-prod-publish", anonymous = TRUE, region = "us-west-2")

      end <- ifelse(grepl("CAT", x, ), "_cat.parquet",
                    ifelse(grep("TOT", x), "_tot_parquet",
                           "_acc.parquet"))

      ds <- arrow::open_dataset(paste0("s3://anonymous@prod-is-usgs-sb-prod-publish/",
                                       id, "/", id, end, "?region=us-west-2"))

      sub <- filter(ds, COMID %in% ids)

      att <- collect(sub)

      att$characteristic_id <- x
      att$percent_nodata <- 0

      select(att, all_of(c(characteristic_id = "characteristic_id", comid = "COMID",
                           characteristic_value = x, percent_nodata = "percent_nodata")))
    })
  )
}

rescale_characteristics <- function(vars, cat_vars_scaled) {
  # assign columns based on the desired summary operation
  cols_sum <- vars$characteristic_id[vars$summary_statistic == "sum"]
  cols_area_wtd_mean <- vars$characteristic_id[vars$summary_statistic == "area_weighted_mean"]
  cols_length_wtd_mean <- vars$characteristic_id[vars$aggregation_statistic == "length_weighted_mean"]
  cols_min <- vars$characteristic_id[vars$summary_statistic == "min"]
  cols_max <- vars$characteristic_id[vars$summary_statistic == "max"]

  # rescale NHDPlusv2 attributes to desired catchments
  # !note that there are currently no adjustments made for the length of split flowlines
  nhd_vars_rescaled <- cat_vars_scaled |>
    group_by(ID) |>
    summarize(
      areasqkm_sum = sum(split_basin_areasqkm),
      lengthkm_sum = sum(lengthkm),
      across(any_of(cols_area_wtd_mean), weighted.mean, w = split_basin_areasqkm, na.rm = TRUE, .names = "{col}_area_wtd"),
      across(any_of(cols_length_wtd_mean), weighted.mean, w = lengthkm, na.rm = TRUE, .names = "{col}_length_wtd"),
      across(any_of(cols_sum), sum, na.rm = TRUE, .names = "{col}_sum"),
      across(any_of(cols_min), min, na.rm = TRUE, .names = "{col}_min"),
      across(any_of(cols_max), max, na.rm = TRUE, .names = "{col}_max")
    ) |>
    ungroup()

  return(nhd_vars_rescaled)
}

# The code below calls a function that grabs the catchment characteristics. I haven't defined
# the function here, but it is included in my previous comment within this GitHub issue.

# Define a function to rescale the catchment attributes using the tabular data.
# This function is a combination of Andrew Sekellick's workflow to accommodate
# split catchments and a function I previously included in this GitHub issue
# to aggregate NHDPlusv2 attributes to new fabrics.

#' @param vars data frame containing two columns: "characteristic_id" is a character
#' vector of desired variables to retrieve from /link{discover_nldi_characteristics};
#' "summary_statistic", is a character vector indicating which summary statistic
#' should be applied to rescale each characteristic. Accepted values include
#' "sum," "length_weighted_mean," "area_weighted_mean," "min," and "max."
#' @param lookup_table data frame containing at least three columns: "ID" is a
#' numeric vector of identifiers at the desired scale; "comid" is a numeric vector
#' of NHDPlusv2 identifiers; "member_comid" contains the formatted NHDPlusv2 COMIDs,
#' for example, in the case of split catchments. If catchments have not been split,
#' the columns "comid" and "member_comid" should be identical.
#' @param refactored_areas data frame containing columns "FEATUREID" and "areasqkm."
#' Used to retrieve adjusted catchment areas in the case of split catchments.
#'
#' @importFrom dplyr left_join rename_with mutate across group_by summarize ungroup distinct
#' @importFrom tidyr pivot_wider
#' @importFrom tidyselect starts_with any_of
#'
rescale_nldi_characteristics <- function(vars, lookup_table, refactored_areas){

  # check that the inputs match what we are expecting
  if(!all(c("characteristic_id", "summary_statistic") %in% names(vars))){
    stop("Check that vars contains columns 'characteristic_id' and 'summary_statistic'")
  }
  summary_stat_acceptable <- vars$summary_statistic %in% c("min", "max", "sum",
                                                           "length_weighted_mean",
                                                           "area_weighted_mean")
  if(!all(summary_stat_acceptable)){
    stop("Check that all entries in vars$summary_statistic match accepted values")
  }
  if(!all(c("comid", "member_comid", "ID") %in% names(lookup_table))){
    stop("Check that lookup_table contains columns 'comid,' 'member_comid,' and 'ID'")
  }

  # omit any duplicated rows in the lookup table
  lookup_table <- distinct(lookup_table)

  # download nldi characteristics for the requested comids and pivot to wide format
  nhd_vars <- get_catchment_characteristics(varname = vars$characteristic_id,
                                            ids = unique(lookup_table$comid))

  nhd_vars_wide <- pivot_wider(nhd_vars, !percent_nodata,
                               names_from = characteristic_id,
                               values_from = characteristic_value)

  # get comid catchment areas, adjusting area for catchments that have been "split"
  nhd_subset <- get_catchment_areas(unique(lookup_table$member_comid), refactored_areas)

  # combine the nldi characteristics with the catchment identifier and basin area
  cat_vars_unscaled <- lookup_table |>
    left_join(y = nhd_subset, by = c("member_comid","comid")) |>
    left_join(y = nhd_vars_wide, by = "comid")

  # rescale the nldi characteristics if needed (i.e., for split catchments)
  if(!all(cat_vars_unscaled$comid == cat_vars_unscaled$member_comid)){
    var_names <- unique(nhd_vars$characteristic_id)
    cat_vars_scaled <- mutate(cat_vars_unscaled, across(any_of(var_names), ~.x*split_area_prop))
  } else {
    cat_vars_scaled <- cat_vars_unscaled
  }

  return(rescale_characteristics(vars, cat_vars_scaled))
}

# Define the function `get_catchment_areas` that's called in `rescale_nldi_characteristics`

#' @title Get basin area and flowline length for NHDPlusV2 COMIDs
#'
#' @description
#' Get catchment area and flowline length for NHDPlusv2 COMID(s) of interest.
#' If any COMID represents a split catchment, the split catchment areas are
#' returned, along with a column that represents the proportion of the original
#' NHDPlusv2 catchment area that is covered by the split catchment.
#'
#' @param comids integer vector or character vector containing NHDPlusV2 identifiers
#' @param refactored_areas data frame containing columns "FEATUREID" and
#' "areasqkm." Used to retrieve adjusted catchment areas in the case of split
#' catchments.
#'
#' @importFrom sf st_drop_geometry
#' @importFrom dplyr mutate select right_join left_join filter rename bind_rows
#'
get_catchment_areas <- function(comids, refactored_areas){

  # format comids (omit suffix "." in the case of split catchments)
  comids_fmt <- data.frame(member_comid = comids) |>
    mutate(comid = as.integer(member_comid))

  # fetch basin area for all comids
  nhd_subset <- nhdplusTools::get_vaa(atts = c("comid", "areasqkm", "lengthkm")) |>
    select(comid, areasqkm, lengthkm) |>
    right_join(x = comids_fmt, by = "comid")

  # handle "split" catchments (if applicable)
  if(all(comids_fmt$member_comid == as.character(comids_fmt$comid))) {
    nhd_subset_out <- nhd_subset |>
      mutate(comid = as.integer(comid))
  } else {
    # create data frame containing the "split" catchments
    comids_to_split <- filter(nhd_subset, grepl(".", member_comid, fixed = TRUE))

    # get the basin area for the split catchments
    split_areas <- refactored_areas |>
      filter(FEATUREID %in% comids_to_split$member_comid) |>
      rename(split_basin_areasqkm = areasqkm)

    split_areas_fmt <- left_join(x = comids_to_split,
                                 y = split_areas,
                                 by = c("member_comid" = "FEATUREID"))

    # create data frame containing the unsplit catchments
    comids_no_split <- filter(nhd_subset, !grepl(".", member_comid, fixed = TRUE))
    unsplit_areas_fmt <- mutate(comids_no_split, split_basin_areasqkm = areasqkm)

    # bind split and unsplit catchments back together
    nhd_subset_out <- bind_rows(unsplit_areas_fmt, split_areas_fmt) |>
      mutate(comid = as.integer(comid),
             split_area_prop = split_basin_areasqkm/areasqkm)
  }

  return(nhd_subset_out)
}

# Apply the functions above to an example.

# First, define the attributes of interest and how they should be
# summarized when aggregated or split.
vars <- data.frame(characteristic_id = c("CAT_EWT","CAT_WILDFIRE_2011", "CAT_BASIN_AREA"),
                   summary_statistic = c("area_weighted_mean", "min","sum"))

# Next, get the lookup table from the refactored hydrofabric that
# contains information about how NHDPlusv2 catchments are split.
file_name <- "refactor_02.gpkg"
file_path <- file.path(tempdir(), file_name)
gpkg_file <- sbtools::item_file_download("61fbfdced34e622189cb1b0a",
                                         destinations = file_path,
                                         names = file_name,
                                         overwrite_file = TRUE)
lookup_table <- sf::st_read(gpkg_file, layer= "lookup_table") |>
  dplyr::rename(comid = NHDPlusV2_COMID, member_comid = member_COMID, ID = reconciled_ID) |>
  # subset lookup table for this example
  dplyr::filter(ID %in% c(10012268, 10012979, 10024047, 10024048, 10024049, 10024050))
lookup_table
# > lookup_table
# comid member_comid       ID LevelPathID
# 1 4146596      4146596 10012268     2132667
# 2 4147382      4147382 10012268     2132667
# 3 4147378      4147378 10012979     2130154
# 4 4147370      4147370 10012979     2130154
# 5 4147396    4147396.1 10024047     2123206
# 6 4147396    4147396.2 10024048     2123206
# 7 4147380    4147380.1 10024049     2123206
# 8 4147380    4147380.2 10024050     2123206

# We will also need the dataset "split_divides" that gives the basin
# area for the split catchments.
split_divides <- sf::st_read(gpkg_file, layer = "split_divides")

# rescale the characteristics to the refactored hydrofabric
rescale_nldi_characteristics(vars, lookup_table, refactored_areas = split_divides)
# A tibble: 6 x 6
#   ID areasqkm_sum lengthkm_sum CAT_EWT_area_wtd CAT_BASIN_AREA_sum CAT_WILDFIRE_2011_min
# <dbl>        <dbl>        <dbl>            <dbl>              <dbl>                 <dbl>
#   1 10012268        12.9          6.30           -25.5               12.9                      0
# 2 10012979         4.78         3.31           -11.9                4.77                     0
# 3 10024047         6.51         4.31           -24.6                6.52                     0
# 4 10024048         1.44         4.31            -5.44               1.44                     0
# 5 10024049         5.84         5.10           -10.8                5.84                     0
# 6 10024050         7.56         5.10           -14.0                7.56                     0
# >

Sort of on topic, for aggregating up to a POI hydrofabric, we'd need to aggregate to the refactor fabric first (with this code) and then aggregate again to the POI fabric right? Don't think we could easily go directly from NHD to a POI fabric...

I like these edits, @dblodgett-usgs, thanks! I haven't used the refactored hydrofabrics much yet - do you have thoughts on @ajsekell's question about the POI fabric? I think that if you had a similar lookup table and a table with the refactored areas you could go from NHD to a POI fabric with this code. Does that sound right?

Sounds good, @dblodgett-usgs. We may start trying to scale this up using the functions in your comment above, and then replace with the nhdplusTools implementation down the line. Also, a random thought - should we note somewhere in the documentation that this process of splitting catchments may not work well for attributes that are not spatially uniform?

I would suggest including a comment like that in the documentation. I'd like to recalculate something like NLCD directly from the rasters to a POI fabric and see if there's a significant difference between that and a NHD level compilation to NHGF POI script.

If you install from the hydroloom branch,
remotes::install_github("doi-usgs/nhdplusTools@hydroloom"),
you'll see I got two new catchment access functions incorporated. I am working on incorporating the rest of the functions above over the next couple days.
See https://github.com/DOI-USGS/nhdplusTools/issues/304#issuecomment-1410784552 for how those functions currently work.

This is now complete in #324 -- @lekoenig can you kick the tires?
remotes::install_github("doi-usgs/nhdplusTools@hydroloom")
??rescale_catchment_characteristics

You can see what I added here: https://github.com/DOI-USGS/nhdplusTools/pull/324/commits/04b195fc9b82d6b05b3d2054838cb6781617a619
Todo on this is to

[ ] deal with some outstanding warnings about deprecated function usage,
[ ] check over documentation and write some examples
[ ] flesh out tests to include some proper unit tests that verify the calculations do what we expect on a onesy twosy basis or just spot check what's in there and verify the numbers in some test expectations.

I'll deal with the warnings but would welcome help on the other two.

To test the rescaling functions I've tried to request characteristics that differ from the ones I was using to develop the code above. I've run into an edge case with CAT_IMPV11 where rescale_catchment_characteristics throws an error:
# rescale NHDPlusv2 attributes by aggregating comid values to new "id" values
vars <- data.frame(characteristic_id = c('CAT_IMPV11',"CAT_SANDAVE", "CAT_BASIN_AREA"),
                   summary_statistic = c("area_weighted_mean", "min","sum"))
lookup_table <- data.frame(id = rep(10012268, 2),
                           comid = c(4146596, 4147382),
                           member_comid = c(4146596, 4147382))
rescale_catchment_characteristics(vars, lookup_table)

Error in `summarize()`:
i In argument: `across(...)`.
i In group 1: `id = 10012268`.
Caused by error in `across()`:
! Can't compute column `CAT_IMPV11_area_wtd`.
Caused by error in `x * w`:
! non-numeric argument to binary operator
Run `rlang::last_error()` to see where the error occurred.
Warning message:
Values from `characteristic_value` are not uniquely identified; output will contain list-cols.
* Use `values_fn = list` to suppress this warning.
* Use `values_fn = {summary_fun}` to summarise duplicates.
* Use the following dplyr code to identify duplicates.
  {data} %>%
  dplyr::group_by(comid, characteristic_id) %>%
  dplyr::summarise(n = dplyr::n(), .groups = "drop") %>%
  dplyr::filter(n > 1L)
>

The dplyr warnings are pretty helpful, actually. There are multiple rows with the same characteristic_id and characteristic_value that cause problems when trying to pivot the data to wide format. This is seen in rows 1 and 3 in the returned data frame below. My guess is that CAT_IMPV11 is included in more than one of the attribute tables and so is getting returned twice. Maybe a simple fix like adding a distinct() call within rescale_catchment_characteristics before pivoting the data to wide format would do the trick?
get_catchment_characteristics(varname = vars$characteristic_id,
                              ids = unique(lookup_table$comid))

   characteristic_id   comid characteristic_value percent_nodata
1:        CAT_IMPV11 4146596                 0.00              0
2:        CAT_IMPV11 4147382                 0.00              0
3:        CAT_IMPV11 4146596                 0.00              0
4:        CAT_IMPV11 4147382                 0.00              0
5:       CAT_SANDAVE 4146596                24.65              0
6:       CAT_SANDAVE 4147382                24.64              0
7:    CAT_BASIN_AREA 4146596                 0.96              0
8:    CAT_BASIN_AREA 4147382                11.98              0
>

Actually, I'm not sure about that. I'm testing river segments in the DRB and it looks like sometimes two different values get returned for CAT_IMPV11. Any idea what's going on there?
vars <- data.frame(characteristic_id = "CAT_IMPV11", summary_statistic = "area_weighted_mean")
lookup_table <- data.frame(id = 1435, comid = 1748535, member_comid = 1748535)
get_catchment_characteristics(varname = vars$characteristic_id, ids = unique(lookup_table$comid))

   characteristic_id   comid characteristic_value percent_nodata
1:        CAT_IMPV11 1748535                 0.00              0
2:        CAT_IMPV11 1748535                 0.25              0

I tracked this down the a bad assumption. The CAT_IMPV11 data has duplicates for these comids.
@mewieczo can you look at what's up with https://www.sciencebase.gov/catalog/item/570577fee4b0d4e2b7571d7b I'm seeing duplicate rows in the cat parquet file. Is that also in the csv or is something wrong with the parquet conversion?
In the mean time, if you update from the hydroloom branch, this should work.
remotes::install_github("doi-usgs/nhdplusTools@hydroloom")
library(nhdplusTools)
vars <- data.frame(characteristic_id = c('CAT_IMPV11',"CAT_SANDAVE", "CAT_BASIN_AREA"),
                   summary_statistic = c("area_weighted_mean", "min","sum"))
lookup_table <- data.frame(id = rep(10012268, 2),
                           comid = c(4146596, 4147382),
                           member_comid = c(4146596, 4147382))
rescale_catchment_characteristics(vars, lookup_table)
#> # A tibble: 1 × 6
#>         id areasqkm_sum lengthkm_sum CAT_IMPV11_area_wtd CAT_BASIN_ARE…¹ CAT_S…²
#>      <dbl>        <dbl>        <dbl>               <dbl>           <dbl>   <dbl>
#> 1 10012268         12.9         6.30                   0            12.9    24.6
#> # … with abbreviated variable names ¹CAT_BASIN_AREA_sum, ²CAT_SANDAVE_min

Created on 2023-02-14 with reprex v2.0.2

Ah, OK - thanks for looking into that IMPV11 issue! I've tested these functions out on a larger population of stream reaches (~15K NHDPlusV2 COMIDs in the DRB) and an expanded set of ~30 attributes to compare against our previous PUMP workflow. Overall, the new nhdplusTools rescale workflow works well! I've added some suggestions for documentation (including examples) and fixes for the dplyr function usage in a branch on my fork. I can open a PR if that fits with your workflow, @dblodgett-usgs.
I've also found a few other potential edge cases that we could consider:

The CAT_sinuosity variable gives me trouble. Based on the error message, is it possible this variable has a different name in the metadata table and the parquet files?

library(nhdplusTools)
lookup_table <- data.frame(id = rep(10012268, 2), comid = c(4146596, 4147382), member_comid = c(4146596, 4147382))
vars <- data.frame(characteristic_id = "CAT_sinuosity", summary_statistic = "length_weighted_mean")
rescale_catchment_characteristics(vars, lookup_table)
#> Error in UseMethod("pivot_wider") :
#>   no applicable method for 'pivot_wider' applied to an object of class "NULL"
#> In addition: Warning message:
#> In get_catchment_characteristics(varname = vars$characteristic_id,  :
#>   Issue getting characteristics. Error was:
#> Problem while evaluating `all_of(...)`.Issue getting characteristics. Error was:
...

For rescaled catchments that only contain zero-area COMIDs, the area-weighted values return NaN. It makes sense, and I think I'm OK with that but thought I'd mention it.

library(nhdplusTools)
vars <- data.frame(characteristic_id = "CAT_STRM_DENS", summary_statistic = "area_weighted_mean")
lookup_table <- data.frame(id = 1721, comid = 4188275, member_comid = 4188275)
rescale_catchment_characteristics(vars, lookup_table)
#> # A tibble: 1 x 4
#>      id areasqkm_sum lengthkm_sum CAT_STRM_DENS_area_wtd
#>   <dbl>        <dbl>        <dbl>                  <dbl>
#> 1  1721            0        0.048                    NaN

Created on 2023-02-15 by the reprex package (v2.0.1)

-9999 values in the characteristics tables cause weird rescaled values, and we should probably replace them with NA before calling rescale_characteristics().

library(nhdplusTools)
vars <- data.frame(characteristic_id = "CAT_EWT", summary_statistic = "area_weighted_mean")
lookup_table <- data.frame(id = 4203,
                           comid = c(8074226,8075830,932040118,932040186,932040187),
                           member_comid = c(8074226,8075830,932040118,932040186,932040187))
get_catchment_characteristics(varname = unique(vars$characteristic_id), ids = lookup_table$comid)
#>    characteristic_id     comid characteristic_value percent_nodata
#> 1:           CAT_EWT   8074226                    0              0
#> 2:           CAT_EWT   8075830                    0              0
#> 3:           CAT_EWT 932040118                    0              0
#> 4:           CAT_EWT 932040186                -9999              0
#> 5:           CAT_EWT 932040187                -9999              0
rescale_catchment_characteristics(vars, lookup_table)
#> # A tibble: 1 x 4
#>      id areasqkm_sum lengthkm_sum CAT_EWT_area_wtd
#>   <dbl>        <dbl>        <dbl>            <dbl>
#> 1  4203         22.8         10.3           -5057.

Created on 2023-02-15 by the reprex package (v2.0.1)

I was working with the split catchments to develop additional unit tests based on expected output. I'm wondering whether we actually want to apply the proportion area correction to all vars at the end of rescale_catchment_characteristics(), or whether it'd be better to make that adjustment within rescale_characteristics() and only for the area-weighted cols. Here's a reprex to show what I mean:

library(nhdplusTools)
library(dplyr)
vars <- data.frame(characteristic_id = c("CAT_ELEV_MIN"),
                   summary_statistic = c("min"))
lookup_table <- data.frame(id = rep(10012268, 2), comid = c(4146596, 4147382), member_comid = c(4146596, 4147382))

# works ok for aggregation only
get_catchment_characteristics(varname = unique(vars$characteristic_id), ids = unique(lookup_table$comid))
#>    characteristic_id   comid characteristic_value percent_nodata
#> 1:      CAT_ELEV_MIN 4146596               796.09              0
#> 2:      CAT_ELEV_MIN 4147382               530.76              0
rescale_catchment_characteristics(vars, lookup_table)
#> # A tibble: 1 x 4
#>         id areasqkm_sum lengthkm_sum CAT_ELEV_MIN_min
#>      <dbl>        <dbl>        <dbl>            <dbl>
#> 1 10012268         12.9         6.30             531.

# but for split catchments?
d <- readRDS(list.files(pattern = "rescale_data.rds", recursive = TRUE, full.names = TRUE))

# smallest value for cat_elev_min between comids belonging to id 10024047 or 10024048 is 487.49
get_catchment_characteristics(varname = unique(vars$characteristic_id), ids = unique(d$lookup_table$comid)) |>
  left_join(d$lookup_table, by = "comid", multiple = "all")
#>   characteristic_id   comid characteristic_value percent_nodata member_comid       id LevelPathID
#> 1:      CAT_ELEV_MIN 4146596               796.09              0      4146596 10012268     2132667
#> 2:      CAT_ELEV_MIN 4147370               616.66              0      4147370 10012979     2130154
#> 3:      CAT_ELEV_MIN 4147378               574.88              0      4147378 10012979     2130154
#> 4:      CAT_ELEV_MIN 4147380               530.24              0    4147380.1 10024049     2123206
#> 5:      CAT_ELEV_MIN 4147380               530.24              0    4147380.2 10024050     2123206
#> 6:      CAT_ELEV_MIN 4147382               530.76              0      4147382 10012268     2132667
#> 7:      CAT_ELEV_MIN 4147396               487.49              0    4147396.1 10024047     2123206
#> 8:      CAT_ELEV_MIN 4147396               487.49              0    4147396.2 10024048     2123206

# yet rescaled value of cat_elev_min for id 10024048 is 88.5
rescale_catchment_characteristics(vars, lookup_table, refactored_areas = split_divides)
#> # A tibble: 1 x 5
#>         id areasqkm_sum lengthkm_sum CAT_ELEV_MIN_min
#>      <dbl>        <dbl>        <dbl>            <dbl>
#> 1 10012268        12.9          6.30            531.
#> 2 10012979         4.78         3.31            575.
#> 3 10024047         6.51         4.31            400.
#> 4 10024048         1.44         4.31             88.5
#> 5 10024049         5.84         5.10            231.
#> 6 10024050         7.56         5.10            299.

Finally, it occurred to me that I've ignored the percent_nodata column from get_catchment_characteristics() up until now. I think it could be useful to retain and possible rescale it. I have an idea to do that if it seems worthwhile.

Thanks for the doco @lekoenig this is getting solid -- I'll keep this issue in my todo queue and look at your comments.
If you think you know how to work in the percent_nodata, that would be awesome. I've kind of been overlooking it as well.

CAT_sinuosity is in the "tot" file for some reason? The metadata does have the information needed for that and that case is now covered.
I found a case where there is no "percent_nodata" (for ACC_STRM_DENS). I will just fill in 0 in that case.
For a divide by 0, I think NaN is the best choice.
I now do att <- dplyr::mutate_all(att, ~ifelse(. == -9999, NA, .)) @mewieczo -- do you know if there are other "nodata" values that I will need to parse out?
The issue you highlight in (4) just looks like a bug? I'm not totally following what's going on though. Do you want to dig further and get that fixed up or should I @lekoenig ?

Reinstall from hydroloom to get the little fixes above.
remotes::install_github("doi-usgs/nhdplusTools@hydroloom")

I now do att <- dplyr::mutate_all(att, ~ifelse(. == -9999, NA, .))

Great, thanks for doing that. If mutate_all is ever deprecated, we could revert to the across usage, e.g. something like: dplyr::mutate(att, dplyr::across(is.numeric, ~dplyr::na_if(., -9999))).
The issue in (4) above is really more of a conceptual question than a bug. For split catchments, should we be rescaling all characteristics? In that example, if a user requests the "min" statistic for the attribute CAT_ELEV_MIN, the value that gets returned is lower than any of the contributing catchments (because we rescaled CAT_ELEV_MIN by the proportion of contributing area). I'm not sure that's what we want, but would appreciate a second set of eyes in thinking this through. To avoid rescaling characteristics by proportional area when the characteristics will be used to calculate "min" or "max" summary statistics, a potential fix could be to move this line:
  # rescale the nldi characteristics if needed (i.e., for split catchments)
  if(!all(lookup_table$comid == lookup_table$member_comid)){
    lookup_table <- mutate(lookup_table, across(any_of(var_names), ~.x*.data$split_area_prop))
  }

from rescale_catchment_characteristics to rescale_characteristics and then only apply that correction to certain columns (instead of var_names):
  # rescale the nldi characteristics if needed (i.e., for split catchments)
  if(!all(lookup_table$comid == lookup_table$member_comid)){
    lookup_table <- mutate(lookup_table, across(any_of(c(cols_area_wtd_mean, cols_sum)), ~.x*.data$split_area_prop))
  }

Does that make sense?

If you think you know how to work in the percent_nodata, that would be awesome. I've kind of been overlooking it as well.

I've suggested some changes in #331 to retain and rescale the percent_nodata values.

@lekoenig -- based on what we have now, do you think this could be closed?

I was just looking at this last night to see if we could close this loop. I think the only thing remaining is to resolve (4) described in https://github.com/DOI-USGS/nhdplusTools/issues/303#issuecomment-1431591474. I'd appreciate if you or @ajsekell could take a look at the example above as a check on my logic, but either way I'll open a PR today with my proposed change.

I think I'm following 4 in #303...yeah that's a problem. You're right the rescaling by proportion of area isn't necessarily needed for all variables (like min elevation, or maybe some of the climate variables too). Your suggestion I think makes sense and I also think some of these details will be important in the help/documentation.

@lekoenig -- based on what we have now, do you think this could be closed?

Following up on this, after the changes merged in #337 I think this issue can be closed. In the next couple months we will be working with this code and scaling up these workflows to generate rescaled attributes for CONUS. I'll open a new issue if we find any bugs or have other questions.

Great. As I have time, I am working on getting hydroloom documented and ready for CRAN. Once it's out, nhdplusTools will start to migrate to that back end on the main branch. Until then, we'll just have to install from the hydroloom branch.

See https://doi-usgs.github.io/nhdplusTools/news/index.html
This is now on the main branch and on its way to CRAN


Clarification please re. thrust command velocity or force or %

I would like some clarification. The linear MPC documentation says:

command/roll_pitch_yawrate_thrust of type mav_msgs/RollPitchYawrateThrust. This is the command to the low level controller. Angles are in rad and thrust is in N.

It publishes the z thrust component in Newtons, however the matlab scripts seems to treat it as velocity.
The falcon2_sysid_roll_pitch_onboard.m has a scaling constant

k_thrust = 0.0019965;

I presume the constant was evaluated using the regression scripts similar to the regression_height_falcon2.m that script scales the joystick input to match vertical velocity.
To confuse it more the DJIInterface::commandRollPitchYawrateThrustCallback that accepts the command seems to have limits (minimum_thrust_ and maximum_thrust_) suggesting it is 0-100%
The falcon2_sysid_height_onboard.m fits the throttle command with transfer function into vertical velocity as well.
So, should the thrust.z be a Force in [N], velocity [m/s] or [%]?
Or is does not really matter as long as as the values are the same for experiments and actual control?
Why I am asking ? I am trying to replicate your experiment with M100 with custom quadrotor frame and N3 controller. I created ros node that sends mav_msgs/RollPitchYawrateThrust with roll/pitch in [rad] and thrust.z as 0...100. I am creating those from /dji_sdk/rc topic.

The attitude/z_velocity is deprecated. It was the first version we tested, now the interface supports attitude thrust. I would stick with attitude thrust in [N].

The instructions for collecting the data https://github.com/ethz-asl/mav_dji_ros_interface/wiki/Dynamic-System-Identification (the table on top) indicate input as dji_sdk/vc_cmd/thrust/* and output as dji_sdk/velocity/* as output. Are you saying those instructions are outdated?
Is there falcon2_sysid_height_onboard.m equivalent dealing with acceleration rather than velocity?

Was thinking some more about it, the scripts for roll, pitch calculate the 1'st and 2'nd order transfer function parameters from roll command to roll taken from imu orientation, same for pitch and yaw rate. angle to angle, angle rate to angle rate. However thrust transfer function parameters are calculated from trust.z command to vertical velocity?
Should that be force to force function I mean from thrust.z to imu.linear_acceleration.z * uav_mass? The  MPC is going to send the thrust.z command as Newtons. Am I missing something here?
@inkyusa Inku, I notice you have been working on those scripts, can you shade some light on this?
regards Maciej

OK, I think I found what it is about. The paper dealing with dynamic system identification mention that the DJI SDK only allows for the command to control velocity and not acceleration.

Stick inputs: The common transmitter inputs are pitch,
roll angles ( rad), yaw rate ( rad/s), and thrust (N). However, the vertical stick input of the platform is velocity (m/s) that permits easier and safer manual flight. To address this different, we use a classic PID vertical position controller alongside linear MPC horizontal position controller.

Is that still the case in the master branch of mav_control_rw? If I understood the code correctly the nonlinear mpc in master branch is using transfer functions of first and second order for roll, pitch and yaw rate, but I can only find the penalty values for the thrust: q_z, q_vz, r_thtust. Do not see any references to PID in the code.
My plan is to use this config: https://github.com/ethz-asl/mav_dji_ros_interface/blob/master/dji_interface/launch/raven_indoor_rovio_nMPC_trj.launch. Plugin the transfer function parameters from roll, pitch, yaw_rate experiments and change the sampling_time parameter to match the 400Hz IMU and MSF pose stream from N3 controller.
If you see any problem with this approach please let me know.

Since we wrote the new interface, we use roll, pitch yaw rate and thrust [N]. Z velocity is not used, and no identification beside roll/pitch dynamics is needed.
I don't think you need to run MSF/NMPC at 400 Hz, that's too much. I would go for 50, max 100 Hz if the serial communication doesn't give issues.

@inkyusa I think we should update the documentation of the SysID to match what we have in this repo.

Thanks @fmina !
What you say matches my understanding of the code and the parameters in the config file.
I also was thinking to drop the frequency, the serial communication copes quite well with 400Hz via USB/Serial converter but the tasks that process the data seems to be heavy on CPU.
What a shame to drop the yaw transfer function in my case it fits the validation data  with 91% accuracy :)
Maciej

Thank you for your support, I decided on a compromise the Visual Odometry works really nice with 400Hz IMU, so I run that and MSF at 400Hz and then publish odometry from MSF at 50Hz (every 8'th frame) to nMPC. The DJI N3 controller need some tuning but it works well, I tested it with step responses in square: https://www.youtube.com/watch?v=3LoL0Xdj7PY
Thank you again!

Since we have changed control input interface from virtual RC that sends roll angle, pitch angle, yaw rate, and vertical velocity to thrust command (all same except sending 0-100% thrust command instead vertical velocity as Mina mentioned).
I created another dynamic system identification page for thrust command and we will update in an early manner.
If this issue resolved, please close it.
Best,


Fix/265

#265
Right now is not really fixed, just a failing unit test.
JSON
[
  {
    "id": 1,
    "player_group": {
      "id": 1,
      "players": [
        {
          "id": 1
        }
      ]
    }
  }
]

Core Data Model

I think a simple solution for this might not be possible.

Here right now we're getting 2 instead of one.
The reason that this is happening is because when Sync starts looking for the second player, it doesn't find it, so it creates a new one.
The reason why it doesn't find it is that there are two reasons:
1.- In the DATAFilter it uses an inverse predicate, so it asks for a player that has a player group, and the already inserted player doesn't have a group.
2.- The DATAFilter fetch doesn't return a result, even if the predicate was nil, because the fetch needs that the relationship is saved first.


change much  231025_0136

👷 Deploy Preview for yzm0518 processing.

Name
Link

🔨 Latest commit
038adf29982a98cb148b4d7f7b534088698a4e46

🔍 Latest deploy log
https://app.netlify.com/sites/yzm0518/deploys/65380152fd4b00000715755c


AJAX JSON String Post to Nancy Function Vb.net

Hello All,
I have inherited a Project from another developer. Its a Web App written in VB.net and has Nancy (Not Sure what version). I have a c# background and little VB experience and never have touched Nancy.
I have a web page that allows me to paste images from a clipboard to attach them into an email. I take the images convert it to a base64 string, and put that into an js array. I was attempting to post the data into a function in my VB code but I keep on getting an empty DynamicDictionary.

I am passing it in as an Object but when I mouse over it, it shows up as a Nancy.DynamicDictionary with nothing inside.
How can I have this come in as the raw JSON String instead of a Nancy.DynamicDictionary? I've tried this to no avail (Get Error 500 in chrome debugger):

Also, I apologize if this is the wrong place to ask questions, I work alone and have been staring at this for too many hours now. The room has no windows.

Have you looked at https://github.com/NancyFx/Nancy/wiki/Taking-a-look-at-the-DynamicDictionary and  https://github.com/NancyFx/Nancy/wiki/Model-binding?
Sorry I can't be more helpful, on the phone.

Just read that last part

The room has no windows.

😂🤣

@sourceship I am not sure what the syntax is in VB but you should not be looking into the p parameter because it is just the route path data, not the request body. In your function CaptureFullImage you can access Me.Request.Body which a Stream that you can read:
Dim reader as StreamReader = new StreamReader(Me.Request.Body)
Dim bodyContentAsJson as String = reader.ReadToEnd()

again, I am not sure the syntax is correct but I hope you get the idea

@khellang  and @Zaid-Ajaj  Thank You So Much for your help!
Zaid The code you left was spot on in VB, was able to get the data from within my array. I really appreciate both of you, this was the small thing holding me up yesterday.
Thanks again guys! Wish you all the best!


Email textbox not optimized for mobile view

Description
The email text box is not optimized for mobile view.
Reproduction

Go to Creative Commons Opensource in mobile view
Scroll to footer
See error.

Expectation
The width of the email text box should be adjusted for the device width.
Screenshots

Additional context

Resolution

[x] I would be interested in resolving this bug.

I would like to work on it @TimidRobot

Hey @nitin-pandita , since you are working on #712 ,shall I take up this issue?

Hey @nitin-pandita , since you are working on #712 ,shall I take up this issue?

sure , go for it

@TimidRobot the fix to this issue is in the creativecommons/vocabulary-styles repository in the following file footer.scss
It needs to be updated and the following code added in the mobile() section:
@include mobile() {
    .subscription {
      .newsletter {
        width: 100%;
      }
   }
}

Should I submit a PR directly in that repository or create an issue there?

@adidevs I would concur with @TimidRobot. There's a fix brewing that would address this differently, but is precisely this Issue in the new changes to Vocabulary. I'd say best to make this status: blocked for now.

Thank you for your feedback @possumbilities and @TimidRobot !

as i see there are no PR for this
i am gonna genrate one
see u

Hey @soustab10 can i work on the above issue?

I want to work on this issue please assign me this issue.

Can I work on this issue.

Hi @soustab10, I would like to work on this. Please assign it to me.

Hi, I created a PR with a fix for this: https://github.com/creativecommons/creativecommons.github.io-source/pull/763
I've tested it locally, please review @TimidRobot


Expand implementation of lax_numpy._quantile to allow the input of a …

Expand implementation of lax_numpy._quantile to allow the input of a tuple as axis argument. Fixes #9277

introduced and changed corresponding numpy code to swap and reshape axis to be quantiled
introduced code to accomodate the reintroduction of those axes if keepdims=True
added testcases

@jakevdp tried to address your comments, let me know what you think and thanks for the review effort.

Last thing: could you please squash all the changes into a single commit (see https://jax.readthedocs.io/en/latest/contributing.html#single-change-commits-and-pull-requests). Once you do, we can start the merge process

Made a bit of a git mess at first, but I think it's fine now.

Thanks for the contribution!


fix: resolve actor create build tags to the env instead of game

Changes

[!WARNING]
This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

#1430  👈 (View in Graphite)
#1429
#1426
#1427
main

This stack of pull requests is managed by Graphite. Learn more about stacking.


MariaDB: cast column is null in result

Operating system (distribution) and version
MacOS 10.15.5
DBeaver version
Version <IP_ADDRESS>007051953
Database name and version
10.4.12-MariaDB-1:10.4.12+maria~buster-log

cast(datum_stunde as time) as stunde shows null?!
When scrolling down to next page (200 rows) and up again then it is shown as expected. Maybe problem with "cast"?!

System information:

Operating system (distribution) and version
MacOS 10.15.5
DBeaver version
Version <IP_ADDRESS>007051953
Java version
Additional extensions

Connection specification:

Database name and version
10.4.12-MariaDB-1:10.4.12+maria~buster-log
Driver name
Do you use tunnels or proxies (SSH, SOCKS, etc)?

Describe the problem you're observing:
Steps to reproduce, if exist:
Include any warning/errors/backtraces from the logs

Hi @MeboGit ,
unfortunately, cannot reproduce it. For what data type issue is happened?

concerned field type is datetime:
CREATE TABLE station_datum_stunde (
calledstationid varchar(25) DEFAULT NULL,
mikrotik_router_id int(1) NOT NULL,
hotspot_id int(11) DEFAULT NULL,
datum date DEFAULT NULL,
datum_stunde datetime DEFAULT NULL,
stunde int(7) DEFAULT NULL,
stunde_string varchar(27) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci DEFAULT NULL,
anzahl bigint(21) NOT NULL,
acctsessiontime_mb decimal(10,2) DEFAULT NULL,
acctinputoctets_mb decimal(10,2) DEFAULT NULL,
acctoutputoctets_mb decimal(10,2) DEFAULT NULL,
KEY idx_station_datum_stunde (hotspot_id,datum,stunde)
) ENGINE=InnoDB DEFAULT CHARSET=utf8

Strange: When changing
cast(datum_stunde as time) as stunde,
to
cast(datum_stunde as time) as x,
then it works as expected.
What could be wrong with "stunde"?

seems that issue is related to the already existing column with the name similar to the alias that was used but with another type
thanks for the help with investigation

Fixed (we had the same issue in PostgreSQL)

verified


Port fuzz tests from djot.lua

Any ideas why the jest timeout (3rd parameter in it) isn't working?
@matklad

I am not sure how it could work -- as far as I know, there's no way to kill a running function in JavaScript. So what that timeout could do is to schedule some function to run later when the timeout is expired, but it can't do anything with already running code. Additionally, if the code monopolizes the event loop, that timeout firing code might not even fire....
Will go read the docs -- the existance of the timout parameter is much more surprising that the fact that it doesn't work )

describe("Fuzz tests", () => {
  it("does not exhibit pathological behavior on random input", async () => {
    for (let i = 1; i <= NUMTESTS; i++) {
      await new Promise(r => setTimeout(r, 0));
      let s = randomstring();
      const ast = parse(s, {warn: (() => {})});
      expect(ast).toBeTruthy();
    }
  }, 10);
});

This does the trick -- timeout only works for asynchronous functions, so adding a dummy sleep there gives the event loop ability to run the aborting code.

That definitely wasn't clear from the documentation!

OK, I don't think this actually works.
describe("Pathological tests", () => {
  for (const testname in tests) {
    it("does not exhibit pathological behavior on " + testname, async () => {
      await new Promise(r => setTimeout(r,0)); // dummy sleep see #6
      const test : string = tests[testname];
      const start = performance.now();
      const ast = parse(test, {warn: ignoreWarnings});
      const end = performance.now();
      expect(ast).toBeTruthy();
      expect(end - start).toBeLessThan(1000);
      console.log(end - start);
    }, 10);
  }
});

Note the timeout is set to 10 ms.  However, the output clearly shows that it is taking > 10 ms for each parse, and the timeout doesn't engage. If I set the timeout to 1ms, it will kick in.

That's because the await point is the only place where timeout can engage I think. That is, I think that if you put the timeout at the end, this will work.
But also, if the goal here is just to test that the thing is fast, I'd maybe just manually measure the time and throw if its too long -- seems more direct and bullet proof that going through the framework

I do want to do that (and just pushed a commit that does).
However, we also need a timeout on the test -- otherwise the tests will hang if we get a pathological case.

Moving it to the end seems to work!

However, we also need a timeout on the test -- otherwise the tests will hang if we get a pathological case.

Yeah, that one I think wouldn't work, unless we introduce await point within the parser itself. The code can only be interrupted at an await point, so, if we end up in 2^n loop and there's no await here, we'll loop until the whole CI job is killed

In parcicular, moving it to the end would kill the test only after we finish parsing

Re-opening.

It seems to me that this is a really basic thing for a test framework to have -- a simple way of interrupting any test that hangs after a certain time.  Why does this seem impossible?

It's ... not a basic thing. On a fundamental level, if there's some code running on a CPU, there's no way to stop it from outside within the process. The only two choices are:

for the running code itself to check for some flag once in a while and to shutdown itself when the flag is set
for the operating system to kill the whole process

Some runtime systems (eg, haskell with asynchronous exceptions) essentially automate the insertions of "flag checking points", but that's a rather odd feature. In two other cases I know (Java's interrupt and pthread_cancel), the current wisdom is that not a great thing to have, and that cancellation points should in general be flagged by the programmer (eg, via explicit .await point).
So, it's not really a defficiency of a testing framework, rather, it's a fundamental limitation of runtime system which doesn't support equivalent of asynchroneous exceptions.
Though, I would say that jest is at a fault for not explaining the limitation of their timeout (that it only works for async functions, and that it only is checked at await points). That's my confusion about the feature existing at all.
Practically, I'd maybe just rely on the CI to kill the process eventually?

Actually, let me check a thing...

Ok, so I think one way we can make this work is by running the body of the test in a worker thread:
https://nodejs.org/api/worker_threads.html#worker-threads
That way, parsing won't monopolise the event loop, and would get the chance for the timeout code to run, and the timeout code would be able to kill the worker

I'd maybe just rely on the CI to kill the process eventually?

The problem is that then we don't get information about the input that caused the hang, which is exactly the information we're trying to get out of the fuzz testing.  (Note that jest won't put any output out to the console, even with console.log, until the test completes.)
The worker threads idea seems worth exploring, but I think it's beyond my current understanding of JS to implement it myself.

I'll close this, because we have the fuzz tests.  They can still give useful negative information (no hangs or excessively long parses), even if they can't give positive information (hangs on string S).  If you or someone else wants to explore the worker thread idea, go for it.


pkgbuild/pkg/observe test fails with repository packages

 Issue by whesse
Originally opened as dart-lang/sdk#20576

Due to a version constraint, pkg/observe test is failing with repo packages.  Marking as failing in the status file pkg/pkgbuild.status.
stderr:
Package observe 0.12.0 does not match >=0.11.0 <0.12.0 derived from:

polymer 0.12.2-dev depends on version >=0.11.0 <0.13.0
polymer_expressions 0.12.1-dev depends on version >=0.11.0 <0.13.0
template_binding 0.12.0+3 depends on version >=0.11.0 <0.12.0

Command[dir_copy]: Copying '/Volumes/data/b/build/slave/pub-mac-be/build/dart/pkg/observe' to '/Volumes/data/b/build/slave/pub-mac-be/build/dart/xcodebuild/ReleaseIA32/generated_pub_package_builds/repo_packages/pkg_observe'.
Took 0:00:00.074638
Command[modify_pubspec]: Adding necessary dependency overrides to '/Volumes/data/b/build/slave/pub-mac-be/build/dart/xcodebuild/ReleaseIA32/generated_pub_package_builds/repo_packages/pkg_observe/pubspec.yaml' (destination = /Volumes/data/b/build/slave/pub-mac-be/build/dart/xcodebuild/ReleaseIA32/generated_pub_package_builds/repo_packages/pkg_observe/pubspec.yaml).
Took 0:00:00.004257
Command[pub_get]: /Volumes/data/b/build/slave/pub-mac-be/build/dart/xcodebuild/ReleaseIA32/dart-sdk/bin/pub get (working directory: /Volumes/data/b/build/slave/pub-mac-be/build/dart/xcodebuild/ReleaseIA32/generated_pub_package_builds/repo_packages/pkg_observe)
Took 0:00:01.563000
If you know the Area-* label, please set it _and_ change status to Triaged.
If you are reporting a problem in the Dart Editor, please open the Help ->
About Dart Editor dialog and add the Dart SDK version number to this
report.  If you prefer, you can use the 'Send Feedback' button in the Dart
Editor to file a report that will contain this information and additional
context helpful for debugging the problem.

 Comment by whesse

cc @sigmundch.

 Comment by jakemac53

This has since been fixed

Added AssumedStale label.


Fix/Improve Linting Process

Expected Behavior
As a developer, when I run make lint as directed by the instructions on developing a component I would expect everything to be green and work.
 When asked in Discord about this it was said it was "normal" and that it's due to the various versions of linters that are in use.
Steps to Reproduce the Problem
Clone components-contrib repository
Run make lint
Release Note

RELEASE NOTE:

This is also a pain point for new contributors to the project as each time they make a change to try to fix the linting in the CICD pipeline they need approvals from maintainers.


Possible bug in MapProxy 1.15.0 and 1.15.1 related to SQLite caches

Context
Description:
When I try to run mapproxy-seed in MapProxy 1.15.0, I get the following error:
  File "/home/USER/projects/development/mapproxy/.venv/bin/mapproxy-seed", line 8, in <module>
    sys.exit(main())
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/script.py", line 367, in main
    return SeedScript()()
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/script.py", line 224, in __call__
    seed_tasks = seed_conf.seeds(seed_names)
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/config.py", line 201, in seeds
    seed_conf = SeedConfiguration(seed_name, seed_conf, self)
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/config.py", line 282, in __init__
    ConfigurationBase.__init__(self, name, conf, seeding_conf)
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/config.py", line 229, in __init__
    self.caches = self._caches()
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/config.py", line 276, in _caches
    caches[cache_name] = self.seeding_conf.cache(cache_name)
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/config.py", line 171, in cache
    for tile_grid, extent, tile_mgr in self.mapproxy_conf.caches[cache_name].caches():
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/util/py.py", line 78, in wrapper
    cache[key] = func(self, *args, **kwargs)
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/config/loader.py", line 1481, in caches
    from mapproxy.cache.tile import TileManager
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/cache/tile.py", line 439
    <<<<<<< Updated upstream
    ^^
SyntaxError: invalid syntax

When I try to seed in 1.15.1 after deleting the cache, the first seeding runs without any problems. However, when I try to repeat the seeding, I get the following error:
  File "/home/USER/projects/development/mapproxy/.venv/bin/mapproxy-seed", line 8, in <module>
    sys.exit(main())
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/script.py", line 367, in main
    return SeedScript()()
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/script.py", line 249, in __call__
    seed(seed_tasks, progress_logger=logger, dry_run=options.dry_run,
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/seeder.py", line 498, in seed
    seed_task(task, concurrency, dry_run, skip_geoms_for_last_levels, progress_logger,
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/seeder.py", line 531, in seed_task
    tile_walker.walk()
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/seeder.py", line 326, in walk
    self._walk(bbox, self.task.levels)
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/seeder.py", line 374, in _walk
    self._walk(sub_bbox, levels, current_level=current_level+1,
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/seeder.py", line 374, in _walk
    self._walk(sub_bbox, levels, current_level=current_level+1,
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/seeder.py", line 374, in _walk
    self._walk(sub_bbox, levels, current_level=current_level+1,
  [Previous line repeated 9 more times]
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/seeder.py", line 394, in _walk
    handle_tiles = [t for t in handle_tiles if
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/seed/seeder.py", line 396, in <listcomp>
    not self.tile_mgr.is_cached(t)]
  File "/home/USER/projects/development/mapproxy/.venv/lib/python3.10/site-packages/mapproxy/cache/tile.py", line 209, in is_cached
    self.cache.load_tile_metadata(tile)
TypeError: MBTilesLevelCache.load_tile_metadata() missing 1 required positional argument: 'dimensions'

In contrast, everything works fine in 1.14.0.
Steps to reproduce:
Install MapProxy 1.15.0 or 1.15.1.
Try to run mapproxy-seed with sqlite cache. The error described above occurs.
Delete the cache and run mapproxy-seed again. The first seeding runs successfully in 1.15.1.
Try to run mapproxy-seed again. The second seeding fails with the error described above.
Expected result:
Seeding should work without any errors.
Actual result:
Errors occur in 1.15.0 and 1.15.1, but not in 1.14.0.
Additional information:
The issue seems to be related to the SQLite caches. It's possible that there's a syntax error in the code for MapProxy 1.15.0, which causes the first error. The second error in 1.15.1 seems to be caused by a missing positional argument in the `MBTilesLevelCache.load
Your Environment

Version used:
Environment name and version: python 3.10.16
Operating System and version: Ubuntu Server 22.04 LTS

This issue from @https://github.com/tobias-brunner could be related:
refresh_before not working anymore
https://github.com/mapproxy/mapproxy/issues/625

fixed in the latest release 1.16.0


Simplify the  PushDownLimit.

Which issue does this PR close?

None. This is a follow-up of #4411 and  #4276.
Rationale for this change
Rewrite the code to:

Add more comments for merge limits
Return None in try_optmize if the plan can't be optimized.
Simplify the code.

What changes are included in this PR?

Are these changes tested?
No functional changes, so don't need new tests.

Are there any user-facing changes?
No.

I plan to review this PR tomorrow -- I apologize for the delay @HaoYang670

Thanks again all!


Automatic handling of HTTP error 429 Too Many Requests

Dropbox may rate-limit API requests in certain conditions by responding with HTTP 429 (I believe OAuth 1.0 apps would get HTTP 503 in the same case).
It would be nice if the library would have an opt-in parameter to automatically backoff in such cases.
Here is an example response:
HTTP/1.1 429 Too Many Requests
Server: nginx
Date: Mon, 06 Apr 2015 15:16:56 GMT
Content-Type: application/json
Transfer-Encoding: chunked
Connection: keep-alive
Retry-After: 60.0

{"error": "Rate limiting oauth_accesses_per_access_token"}

As you can see, the response header includes "Retry-After" header, which indicates when it would be safe to repeat the request.
http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.37

See Wiki for usage


[fuzzing] suppress leak in libc

Motivation
CI generates mem leak warnings in libc fast_thread_local that are not
reproducible in dev env or google's oss-fuzz.  Suppress LeakSanitizer
warnings in libc for now.
Test Plan
Canary in CI.

@bors-libra r+
good for now, but let's keep this in mind

:pushpin: Commit 7a8b1b7 has been approved by mimoo

:hourglass: Testing commit 7a8b1b781cbffbb2b52244ea81e5e63e01eade51 with merge 416db6273850793578a79c2ace92fd35c96bb558...

:broken_heart: Test failed - checks-circle_commit_workflow

@bors-libra retry

:hourglass: Testing commit 7a8b1b781cbffbb2b52244ea81e5e63e01eade51 with merge fc4e17686d239c2957aae055e46b5eca05c674a2...

:broken_heart: Test failed - checks-circle_commit_workflow

@bors-libra retry

:hourglass: Testing commit 7a8b1b781cbffbb2b52244ea81e5e63e01eade51 with merge 235db412fabffb398d1f827a9b05aa45c72cd2b4...

:sunny: Test successful - checks-actions_land_blocking_test, checks-circle_commit_workflow
Approved by: mimoo
Pushing 235db412fabffb398d1f827a9b05aa45c72cd2b4 to master...


fix mis-statement on storage engine page

Originally: https://github.com/influxdb/influxdb.com/issues/467
From https://github.com/influxdb/influxdb/issues/4734#issuecomment-155876205, we need to remove that statement from the storage engine page.

closing in favor of #89


Replace os.path with pathlib

pathlib offers a simpler and more human-friendly way of dealing with paths and it seems like interrogate mostly uses os.path.
I believe changing this would be more in line with CONTRIBUTING.md :wink:

interrogate tries to take advantage of modern features whenever possible

I'd be happy to work on this if you find this issue reasonable :)

Hey @trag1c - certainly would welcome a patch for this. I can't remember why os.path was originally chosen, it may have been something to do with Windows machines. Or I might be making it up. Either way, so long as it works on Windows + Unix, I'd appreciate it!

Roger that! Though I think I'll wait for #178 (where I've left a few tips ^^) to merge so that it's easier to me to make the transition 😄

#178 is all set (which you've probably seen) - sorry for the lag!


Change to not translate unique names

Same as #23964
The proper noun is translated in localized version. So, I want you to use a code fence not to translate.

@changeworld : Thanks for your contribution! The author, @mgoedtel, has been notified to review your proposed change.

@changeworld - Thanks for your contribution. I've reviewed and approve your changes #sign-off.  Thanks.


Adds first methods to dlxApi object

Especially check out addPermission and see if it's appropriate

Within each subsection, can you order the methods alphabetically? And order the subsections alphabetically too? I try to do alphabetical unless there's some other reason not to, just to make things easier to find.

@dwhieb I made another commit, hopefully addressing all parts of your code review, and it looks like this pull request automatically updated, so take another look and let me know what you think

Man, keep discovering all sorts of nifty things about GitHub...
I'll look it over now! Thanks!

Ok, everything looks great! I forgot to mention one thing though: upsert, which adds the item if that item doesn't already exit in the database, or updates the item if it does. So it combines add and update functionality.
I pretty much always use an upsert function, even when I could use an add function. I've never really seen a reason for having separate add functions if you already have an upsert function. So why don't we change all the add functions to upsert?
I'll merge the commits once you're done.

@dwhieb updated "add"s to "upsert"s!

woot! I'll leave the dev branch active so we can continue developing on it going forward.
Next up: Figuring out permissions. Take a look at this issue and let me know what you think.


Add SimpleMenuPreference by @RikkaW

Refactor @RikkaW's SimpleMenuPreference into Android-Support-Preference-V7-Fix. I'm not sure if @RikkaW is okay with this.
Changes: SimpleMenuPreference.java:49 hardcoded popupStyle. May need changing later.
Source: https://github.com/RikkaW/MaterialPreference/tree/2a53e0f191213c3528c60aa243125f7db8f0a8ad/preference-simplemenu
Only tested on Android API level 27.

Copying it over should be okay since it's under Apache v2 license. I'll test it on older APIs and see about the popupStyle though.

Is it gonna take another nine months to get merged? 😢

Not really, I've just had some work to do.

Tested on Android 4.4. Should be good enough to merge.

Does it have to fall back to ListPreference on pre-Lollipop? Haven't had time to test it yet. Which part requires API 21+?

You mean you want it to fallback to DropDownPreference instead? That should be doable as well.

No, I meant why can't pre-Lollipop display it just like an API 21+ device? With the simple menu style and whatnot. Is there a particular call / view / style that requires API 21+?

Okay I did a quick test. It turns out that it's missing elevation and enter animation. Everything else works fine on Android 4.4.

Great. Only 8.25 months to go. 😃

Testing now

Merged. It will be released in v<IP_ADDRESS>.


COCO Dataset Config Files

Hi, can you share the configuration file for the COCO dataset?

Hi, sorry for the delayed reply. I haven't tried this repo on COCO yet.

Hi, sorry for the delayed reply. I haven't tried this repo on COCO yet.


Feature request: run tests in same tab instead of separate browser instances

Currently testing a responsive component or page layout across multiple breakpoints requires a separate headless browser instance.
It might be much more efficient to resize the same browser window instead of spawning a new instance for each test. This would reduce test execution times dramatically.

window resize should not take that long. Can you run them in non-headless mode and see what is going on?

BTW, I am working on a new feature that will boost the test execution a lot (like 20 times faster). Still at POC level but planning to release it soon. So if the current performance does not bother you much you can wait for that

Non-headless mode opens a single browser, but launches each test in a new tab - so the load time is still lengthy.
I tried removing .newPage() for subsequent tests to attempt to force the test in the same tab, but was getting strange test results.  For example, the first test passed showing correct styles, subsequent tests gave false positives (logo on the right should not be appearing):
it("appear correctly on large devices", async () => {
  await differencify
    .init()
    .newPage()

    .setViewport(getViewport('large', VIEWPORT_HEIGHT))
    .goto(TEST_HOST + PATH)
    .waitFor(DELAY)

    .screenshot()
    .toMatchSnapshot()
    .close()
    .end();
});

it("appear correctly on medium devices", async () => {
  await differencify
    .init()
    // .newPage()

    .setViewport(getViewport('medium', VIEWPORT_HEIGHT))
    .goto(TEST_HOST + PATH)
    .waitFor(DELAY)

    .screenshot()
    .toMatchSnapshot()
    .close()
    .end();
});

Ideally testing a simple viewport change could be run in the same tab.
Great to hear about he POC speed boost. Loving Differencify, just looking at ways we can get the most out of it.


Support kernel disassembly

From @D0ntPanic on May 20, 2015 3:25
Instructions such as in, out, hlt, etc. are currently marked as instructions that end disassembly to prevent unnecessarily large disassembly graphs when ROP gadget hunting or in anti-disassembly code. A kernel disassembly mode should be available when disassembling code that is meant to be run in kernel mode, as these instructions are allowed and expected there.
Copied from original issue: Vector35/binaryninja#34

Just realized we missed this one, we should have support for both privileged instructions and loading kernel modules. Closing as complete.


Defining "Framing" - about 7 years ago I made the statement on /r/psychonaut "it's not the picture, it's the frame" let me explain.

When you're tripping, it's common for your field of view to expand, you see out of your peripherals further and clearer. The same even applies in closed-eye-visual states. Sometimes you can have an OBE where the peripherals continually forever expand. What this does is it makes the picture that was once there in the middle, a tiny tiny view within the frame of what was there before. Meaning that if you were to think of art piece, say Picasso, then it becomes that it's more-so the surrounding environment that builds the beauty of the picture.
Similarly, "Framing" can also be used to refer to essentially listening to someone speak or reading a sentence about a story, or even hearing it second hand, you take out and take note of certain things within their statements and tend to pull out only what you attached to. Then when you pass the information, that they passed to you, onwards, then you end up re-framing what they said. And sometimes this can be good, sometimes this can be bad, sometimes (usually all the time) there are pros and cons within everything, as well as much much much more. But this is not the end of that process. Framing is infinite and eternal, a continuous chain of forever, passing through and paying it forward, or taking it back.
When someone hears the same story you heard, but they frame it differently than you, you should not be alarmed that they saw something different in it. This is framing in a nutshell, it is ultimately synchronistic (read: co-incident + meaningful - ie Bayesian Impact Theory) and dependent on your attachments. You can only do you.

Sorry I'm struggling to see how this is relevant to OSPAT. It is missing any suggestions or content for the OSPAT repo. Please refrain from submitting issues without sufficient justification.

Have you not read your own repo?
https://github.com/unshakenme/OSPAT/blob/main/course-content/navigating-psychedelic-experience/Framing - BurnZero.pdf

Admittedly I had not read that yet. Cheekeymonkey is a trust contributor so it was a brief review when he submitted that pull request.

@unshakenme so tell me why should I contribute to an empty repo?


Kelly Card died

store:
  backend: mongo
  # address or path for the database
  address: "user:passwd@<IP_ADDRESS>:27017"
  # open database in read-only mode
  read_only: false
  # backend-specific options
  options:
    database_name: cayley
query:
  timeout: 30s
load:
  ignore_duplicates: false
  ignore_missing: false
  batch: 100000

cayley version: 0.75
After the data is imported for a while or the piece is gone, the process is gone and the program is dead.

Can you please provide a log for this?

Closing for no response. Can reopen if @xingxing9688 will respond.


2D Offset breaks due to overlapping wires

Minimal reproducing example:
from build123d import *

sk = Rectangle(100, 100) - GridLocations(80, 80, 2, 2) * Circle(5)
part = extrude(sk, 10)

sk2 = offset((part.faces() >> Axis.Z).first, -5)
part -= extrude(sk2, -5)

creates the following (broken) part:

Replacing the offset function with the following safe_offset2D function
def safe_offset2d(face: Face, offset: float):
    outer = face.outer_wire().offset_2d(offset)
    inners = [inner.offset_2d(-offset) for inner in face.inner_wires()]
    new_face = Face(outer)
    for inner in inners:
        new_face -= Face(inner)
    return new_face

yields the desired result:

The culprit is likely to be https://github.com/gumyr/build123d/blob/dev/src/build123d/operations_generic.py#L619 where a new Face is created from the offset outer and inner wires, without checking for potential overlap between them.

Fixed:
sk = Rectangle(100, 100) - GridLocations(80, 80, 2, 2) * Circle(5)
sk2 = sk - offset(sk, -5)


sticker-242, collection-set-community-33 is null on name

I can't find the collection-set-community-33

Never mind I found it.
I have fixed the error with sticker-242 name's. The second error I am not sure what they are doing.

set_community_33 is the same as the set_community_32

Will leave it like that and wait for new updates to clarify all this. If you have a better idea let me know.

Maybe now want skip skin, collection have null name for parse.

But on skin have null name have different paint_index "collection-set-community-32" vs "collection-set-community-33"

Okay, I will skip all the collections and skins that have null name to avoid errors.


AbsoluteExpiry of caller overridden/ignored if child called that causes cache entry with shorter expiry

If method A caches for 12 hours and calls method B which caches for 5 minutes, the result for method A will only be cached for 5 minutes.
Logically this appears to make sense but this happens without the caller for Method A knowing about it. Consider the situation where method A goes on to make calls to a system that has a quota that mustn't be called too frequently. There should at least be an error if Method A has set an absolute expiry that is subsequently ignored/overridden by the call to Method B.
It relates to this change:
https://github.com/aspnet/Caching/commit/f15fb804cdc2e14f9a64896817f3c6c343110820#diff-9968fc5543acfd05fa60089bd344679bR102
The actual behaviour appears to contradict the bullet point above but that could depend on your interpretation of which is child and which is parent in the scenario I've described: "When one cache entry is used to create another, the child copies the parent entry's expiration tokens and time-based expiration settings. The child isn't expired by manual removal or updating of the parent entry."
I've put a code example here: https://sartorialsolutions.wordpress.com/2019/02/15/aspnetcore-memorycache-magic-dependencies-and-danger/

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 13f5f8f9-18dd-9c12-9ed1-fcc6b44ba7fd
Version Independent ID: 640b4f36-5886-7b69-1c95-cfc8574a02fd
Content: Cache in-memory in ASP.NET Core
Content Source: aspnetcore/performance/caching/memory.md
Product: aspnet-core
GitHub Login: @Rick-Anderson
Microsoft Alias: riande

there should at least be an error if Method A has set an absolute expiry

You can open a issue for that at https://github.com/aspnet/AspNetCore/issues

The actual behaviour appears to contradict the bullet point above

What bullet point in the doc? What would you like see changed in the doc?

Thanks for the response @Rick-Anderson. The bullet point was the part I quoted (it was "above" when I created this issue on https://docs.microsoft.com/...):
"When one cache entry is used to create another, the child copies the parent entry’s expiration tokens and time-based expiration settings. The child isn’t expired by manual removal or updating of the parent entry."
But perhaps this is talking about an entirely different scenario of creating child cache items.

@ericstj another caching related issue.

cc @maryamariyan @eerhardt

Also cc @sebastienros who made the change referenced here.

@mkArtakMSFT  https://github.com/dotnet/aspnetcore-internal/wiki/Team-Ownership doesn't say who own's caching so adding label re-Artak, feel free to use another label

Fixed and rerouted to @adityamandaleeka