Skip to content

use_as_rownames function #8#11

Merged
krlmlr merged 11 commits intotidyverse:masterfrom
zhilongjia:master
Mar 8, 2016
Merged

use_as_rownames function #8#11
krlmlr merged 11 commits intotidyverse:masterfrom
zhilongjia:master

Conversation

@zhilongjia
Copy link
Copy Markdown
Contributor

use_as_rownames function returns data.frame, not tbl, with rownames. This is to solve issue #8.

Closes #8.

Closes #28.

R/dataframe.R Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd prefer if the two functions were documented together (via @rdname).

@codecov-io
Copy link
Copy Markdown

Current coverage is 96.57%

Merging #11 into master will increase coverage by +0.13% as of f2e0c32

@@            master     #11   diff @@
======================================
  Files           11      11       
  Stmts          422     438    +16
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit            407     423    +16
  Partial          0       0       
  Missed          15      15       

Review entire Coverage Diff as of f2e0c32

Powered by Codecov. Updated on successful CI builds.

R/dataframe.R Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this is intended behavior, it should be documented, but I wonder if it's necessary. (One can always write use_as_rownames() %>% as.data.frame or the other way round, if necessary.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

At least to me, use_as_rownames is to interact with the base::data.frame and tbl is trying to avoid rownames. So I convert it into data.frame directly. Comments?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense; still, needs to be documented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do not get meaning of to be documented. Return a data.frame is documented in details now. Correct?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missed that, sorry.

@zhilongjia
Copy link
Copy Markdown
Contributor Author

add_rownames keeps row names of data frame (Probably it affects a lots). And it will stop if there is a var column names in the input data frame.

More tests are added (I use test a few. Maybe need update).

use_as_rownames will update the row names using var whether it exists or not,then delete the var column. And it returns data.frame so far.

R/dataframe.R Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should go into the @details section

@krlmlr
Copy link
Copy Markdown
Member

krlmlr commented Dec 18, 2015

Thanks. This looks good, I'll merge it soon.

DESCRIPTION Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This spurious change prevents me from merging into master.

R/dataframe.R Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The new function isn't supposed to convert to tbl_df. Let's remove add_rownames() altogether and keep it only in dplyr for compatibility.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rownames_to_column is a copy of add_rownames. So you mean keep the input of rownames_to_column as it is (I did not change that section)? For example, input data.frame output data.frame? I will remove add_rownames.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, data.frame -> data.frame, tbl_df -> tbl_df. Thanks.

@krlmlr krlmlr removed the question label Mar 2, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove add_rownames() add_rownames enhancement

4 participants