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

Create clean_names() method for sf objects #249

Merged
merged 13 commits into from Nov 25, 2018

Conversation

Projects
None yet
2 participants
@JosiahParry
Copy link
Contributor

JosiahParry commented Nov 1, 2018

Based on previous input and the awesome make_clean_names() function, this pull request cleans all names but the geometry feature for sf objects. It is exceptionally light weight and causes no errors on my testing.

JosiahParry and others added some commits Oct 26, 2018

@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 1, 2018

Codecov Report

Merging #249 into master will decrease coverage by 0.29%.
The diff coverage is 81.81%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #249     +/-   ##
========================================
- Coverage     100%   99.7%   -0.3%     
========================================
  Files          22      22             
  Lines         663     673     +10     
========================================
+ Hits          663     671      +8     
- Misses          0       2      +2
Impacted Files Coverage Δ
R/clean_names.R 83.33% <81.81%> (-16.67%) ⬇️

JosiahParry added some commits Nov 20, 2018

if(!is.data.frame(dat)){stop( "clean_names() must be called on a data.frame. Consider janitor::make_clean_names() for other cases of manipulating vectors of names.") }
stats::setNames(dat, make_clean_names(names(dat), case = case))
# create new clean_names method
clean_names <- function(dat, case) {

This comment has been minimized.

@JosiahParry

JosiahParry Nov 20, 2018

Author Contributor

@sfirke Hey, I finally got around to making some changes. In order to make an sf method, I had to make a default method for clean_names() (which still only accepts adata.frame), so I could then make a sf method.


#------------------------------------------------------------------------------#
#---------------------------- Tests for sf method -----------------------------#

This comment has been minimized.

@JosiahParry

JosiahParry Nov 20, 2018

Author Contributor

@sfirke here are the tests for the sf method of clean_names(). I just changed thetest_df to be a sf object and ran the same tests. Everything checks out :) 👍

@sfirke

This comment has been minimized.

Copy link
Owner

sfirke commented Nov 25, 2018

Looks good, I'm just trying to get everything passing checks which is a pain b/c of the geospatial dependencies. I got check() passing on my computer, now trying to get the right dependencies specified on Travis CI to get GDAL installed. Then I think we can merge.

@sfirke

This comment has been minimized.

Copy link
Owner

sfirke commented Nov 25, 2018

btw I added some commits based on advice I got for doing this, incl. adding sf to Suggests and some code I don't fully understand to register the S3 method upon package loading if sf is available. Take a look at my commits and let me know if anything seems off to you?

oldrel + linux is failing b/c of geo dependency issues ... so remove it
no error message I see, behavior is bizarre, removing from testing for now
@sfirke

This comment has been minimized.

Copy link
Owner

sfirke commented Nov 25, 2018

I don't know how to make a test for the error that occurs when the sf package is not present. I'll open a new issue for that. Otherwise this looks good and I'll merge. Please try it out and see if there's any room for improvement w/ how it handles sf objects!

@sfirke sfirke merged commit 4bf8ddf into sfirke:master Nov 25, 2018

1 of 3 checks passed

codecov/patch 81.81% of diff hit (target 100%)
Details
codecov/project 99.7% (-0.3%) compared to a80d736
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment