Updated base/utils #2758

[moki1202]

fixed tidyverse notes for the remaining variables in base/utils
convert.input :'id'
summarize.result: 'n' , 'statname'

[ashiklom]

A few other places where @infotroph 's comment about .data on the left side of expressions applies.

[moki1202]

@ashiklom

what about the 'n' inside sqrt() function . it will need a .data pronoun as well right?

[ashiklom]

@moki1202 Yes, good find. I suspect that line should have both .data$mean and .data$sd.

[moki1202]

@ashiklom why didn't these variables ( 'mean' and 'sd' in this case ) throw a R CMD check error? because I didn't see them in the Rcheck_reference.log file.

[ashiklom]

why didn't these variables ( 'mean' and 'sd' in this case ) throw a R CMD check error? because I didn't see them in the Rcheck_reference.log file.

Good question! I'm pretty sure it's because mean and sd are pre-defined functions in base R, so the error that No object "mean" / "sd" exists in the global environment wouldn't actually apply! To R CMD check, it looks like you are referring to R's default mean and sd functions, which is a perfectly valid thing to do (even though that's not actually what the code is doing). So basically, it's a coincidence!

We don't strictly speaking need to add the .data pronoun here to address the R CMD check comments, but it's good to be consistent, especially to make things easier for new contributors.

[moki1202]

@ashiklom so if I use '.data$' on every variable used inside "dplyr" functions, it wouldn't harm the overall result?

[ashiklom]

so if I use '.data$' on every variable used inside "dplyr" functions, it wouldn't harm the overall result?

That's correct unless the target variable really is something outside of the data. For example, consider the following two cases:

my_constant <- 8.3145
my_data %>%
  mutate(new_variable = .data$old_variable * my_constant)

my_data %>%
  filter(.data$some_column < my_constant)

In both cases, my_constant really is an object outside of the data frame that I want to use inside the expression.

Fortunately, R CMD check will (correctly) mark both of these cases as valid. That's why it's generally a good idea to limit this to the R CMD check warnings. The mean and sd cases above were places I happened to recognize as needing this too.

[infotroph]

This is looking very good! One indentation fix and one place we want a function (stats::sd) rather than ..data$sd.

Once these changes are made, would you be willing to also delete the warnings that have been fixed from base/utils/test/Rcheck_reference.log? That will make sure the build breaks if anyone adds the same warnings back in a future commit.

[infotroph]

Suggested change

	nc_var <- PEcAn.utils::mstmip_vars[PEcAn.utils::mstmip_vars$Variable.Name == name, ]
	nc_var <- PEcAn.utils::mstmip_vars[PEcAn.utils::mstmip_vars$Variable.Name == name, ]

(To keep the code indented inside the function definition... and assuming my eyes aren't lying to me about the current indent!)

[moki1202]

@infotroph updated both these files.

[infotroph]

Suggested change

	stat = stats::.data$sd(.data$mean) / sqrt(length(.data$n)),
	stat = stats::sd(.data$mean) / sqrt(length(.data$n)),

We actually do want the stats::sd function here rather than a variable in the data

[infotroph]

I took the liberty of committing my last three suggestions right away, so I could see whether they were enough to make the CI build pass:

• Spacing fix: this is the same change you already committed, no idea why it wasn't showing up 🤷‍♂️
• Re-added warning to Rcheck_reference.log about id in convert.inputs, since it's not fixed yet. Cleaning up that file can be a PR of its own!
• removed stray text at end of file

The remaining build error on 4.0.2 looks like a transient package manager issue -- the checks for the utils package all pass before it reaches the error -- so I think this PR is good to merge as it is.

[moki1202]

@infotroph thankyou. I will look into the convert.inputs file once again just to verify that I didn't leave a variable unassigned. Also if I wanted to apply for GSoC this summer, what other tools would you recommend me to learn prior to applying?

[infotroph]

@moki1202 That depends what kind of project you're interested in working on!

We've listed some of the major prerequisites for each of the projects in our [list of GSoC project ideas](https://pecanproject.github.io/gsoc_ideas.html], but in broad strokes we're always looking for people who have skill/interest in one or more of:

• Programming and package development in R (R, devtools, renv, ...)
• Unix/sysadmin tools (bash, make, HPC, ...)
• Data provenance, storage, transformations, and databases (postgres, netcdf, ...)
• Web/API design and programming (HTML, Flask, REST, small amounts JS, Shiny, ...)
• Containerization and orchestration (Docker, kubernetes, Singularity, ...)
• Visualizing data (ggplot, Shiny, HTML, ...)
• Unit and integration testing (testthat, GitHub Actions, ...)
• And of course the whole PEcAn project exists to facilitate ecosystem modeling, so it helps (but is definitely not necessary!) to have some familiarity with the models we support and what they do.

This list is an "any of these areas" not an "all of these" -- nobody on the team is an expert in all of them -- and if you bring strong skills from any one group we can help you learn what you need from the others. If this skills list looks like you but you don't see anything that excites you in the project ideas, join the PEcAn Slack community and ask around -- we can probably point you toward outstanding issues that could be developed into your own project proposal.

[moki1202]

@ashiklom @infotroph if there's still some errors left please update me , ill try to fix them ASAP.

[moki1202]

@ashiklom @infotroph what about this issue? What's causing the 1 failed test?

[infotroph]

@moki1202 It looks like the test failure here is intermittent trouble with the Rstudio package manager, and not related to your changes. I think this is ready to merge as it is.