Skip to content

Conversation

@seandavi
Copy link
Member

@seandavi seandavi commented Jun 2, 2025

Adds:

  1. The function itself
  2. Test
  3. Documentation

This should close Add function to return CFDE funding opportunity numbers #16.
@seandavi seandavi requested review from Copilot and the-mayer June 2, 2025 20:46
@seandavi seandavi self-assigned this Jun 2, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new utility to scrape CFDE funding opportunity numbers, along with associated tests and documentation.

  • Implements cfde_opportunity_numbers() function
  • Adds unit tests for return type and pattern matching
  • Provides man page and updates NAMESPACE to export/import the new function

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
R/cfde_opportunity_numbers.R Implements the HTML scraping logic to extract opportunity numbers
man/cfde_opportunity_numbers.Rd Adds Rd documentation for cfde_opportunity_numbers()
tests/testthat/test_cfde_opportunity_numbers.R Adds tests verifying output type and pattern matching
NAMESPACE Exports the new function and imports necessary rvest functions
Comments suppressed due to low confidence (2)

tests/testthat/test_cfde_opportunity_numbers.R:1

  • [nitpick] Currently there are no tests covering the url parameter behavior (e.g., custom URLs or error handling). Consider adding tests for non-default URLs or simulating missing elements to improve coverage.
test_that("cfde_opportunity_numbers returns a character vector", {

NAMESPACE:39

  • [nitpick] Since the code uses rvest:: prefixes, these manual importFrom(rvest,…) entries are redundant. Either remove the imports or switch to unqualified calls for consistency.
importFrom(rvest,html_attr)

@seandavi seandavi added the enhancement New feature or request label Jun 2, 2025
@seandavi seandavi removed their assignment Jun 2, 2025
seandavi and others added 2 commits June 2, 2025 16:51
Avoid returning  NAs since those are not valuable....

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@the-mayer the-mayer merged commit 78b29e1 into main Dec 3, 2025
1 check passed
@the-mayer the-mayer deleted the feat/cfde-opp-nums branch December 3, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants