Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combine dynamic and static polar plotting functions #64

Merged
merged 23 commits into from
Apr 18, 2024

Conversation

jack-davison
Copy link
Collaborator

@jack-davison jack-davison commented Apr 15, 2024

This PR closes #59, but evolved considerably from it.

In short, it deprecates polarMapStatic() in favour of a static = TRUE argument on polarMap().

There are a few (I think) good reasons for doing this:

  • Easy toggling between dynamic and static maps.
  • Consistent API for users.
  • Less scope for oversights/bugs
  • Alignment with the design of {openair}, which arguably prefers fewer functions with more arguments over more functions with fewer arguments.

The evolution of this has extended to:

  • A significant refinement of function documentation.
  • Consistency of the use of type across nearly all functions (e.g., trajMap()).
  • Ensuring type is always passed to openair::cutData() (was not in trajMapStatic()).
  • Simplification of buildPopup()
devtools::load_all()
#> ℹ Loading openairmaps

polarMap(polar_data, pollutant = "pm10", static = TRUE)
#> ℹ Assuming latitude is 'lat'
#> ℹ Assuming longitude is 'lon'
#> Warning in st_point_on_surface.sfc(sf::st_zm(x)): st_point_on_surface may not
#> give correct results for longitude/latitude data
#> Loading required namespace: raster

Created on 2024-04-15 with reprex v2.0.2

@jack-davison jack-davison marked this pull request as ready for review April 18, 2024 08:04
@jack-davison jack-davison merged commit 3de76fc into master Apr 18, 2024
6 checks passed
@jack-davison jack-davison deleted the feat/combine-polars branch April 18, 2024 08:33
jack-davison added a commit that referenced this pull request Aug 8, 2024
Combine dynamic and static polar plotting functions
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.

[Feature Request]: Combine leaflet & static functions (?)
2 participants