Skip to content

Conversation

@nllong
Copy link
Member

@nllong nllong commented May 9, 2025

  • Make a weather_data_fetcher class for testing
  • Fix the XML to be the correct format. The main issue is that the timestamps were not contiguous, they went from 2022-12 to 2022-01, instead of 2022-12 to 2023-01. This might just be an ordering issue on writing out the values.

see c2a220f#diff-6c40cc6729b8e63472133cbd4dea203250af8e30877087f6b25170c15e823eebR73, you don't need to fix the IntervalFrequency and you don't really need the EndTimeStamp.

You don't need to pull in these changes to run the latest bsyncr-server, fixing the XML output from SEED should suffice.

@nllong nllong requested a review from Copilot May 9, 2025 08:11
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 weather_data_fetcher class to encapsulate weather data retrieval from NOAA and updates associated tests and XML examples to ensure correct data handling. Key changes include:

  • Adding the weather_data_fetcher R6 class in R/weather_data_fetcher.R.
  • Updating tests in tests/ to validate the new weather_data_fetcher functionality and ensuring proper XML format in test data.
  • Refactoring utility functions in R/bsync_utils.R and updating documentation and configuration files.

Reviewed Changes

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

Show a summary per file
File Description
tests/test_weather_data_fetcher.R Added tests for verifying weather_data_fetcher behavior and error conditions.
tests/test_bsyncr.R Introduced an additional test case for dataframe creation using the new XML file.
tests/data/ex_bsync_2.xml Updated the test XML to the correct format.
tests/bsyncr_example.Rmd Revised example Rmd to reference ex_bsync_2.xml and integrate weather_data_fetcher usage.
cspell.json Updated spelling dictionary entries for new terms.
README.md Modified installation instructions to include custom CRAN repo for styler.
R/weather_data_fetcher.R Added the weather_data_fetcher class to fetch and process weather data using rnoaa functions.
R/bsync_utils.R Refactored XML parsing to utilize weather_data_fetcher for retrieving weather data.
NAMESPACE Adjusted exports accordingly.
.lintr Created new linter configuration for consistent style rules.

@nllong nllong requested a review from kflemin May 9, 2025 08:15
nllong and others added 2 commits May 9, 2025 10:16
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@kflemin kflemin merged commit 5855b8a into develop May 9, 2025
2 checks passed
@kflemin kflemin deleted the make-weather-class branch May 9, 2025 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants