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

rename "data" generic? #37

Closed
levisc8 opened this issue Nov 15, 2018 · 3 comments
Closed

rename "data" generic? #37

levisc8 opened this issue Nov 15, 2018 · 3 comments

Comments

@levisc8
Copy link
Collaborator

levisc8 commented Nov 15, 2018

I noted that #32 introduces a data generic to extract the metadata w/ list column from the CompadreData structure. However, data() is also a function from utils used to load packages' internal data sets (e.g. data(mtcars)).

I don't think it would cause any actual problems to have the method named as such, but it may be confusing for users interpreting code that employs it because it doesn't really do the same thing as the utils function and that is often used in teaching materials (e.g. vignettes, online tutorials, etc). maybe calling it extract_data or something is safer?

@iainmstott
Copy link
Collaborator

I noticed this once I'd sent the pull request, and I agree... I think actually I favour getting rid of it altogether: originally I put it in to give access to the data slot, but as I mentioned in #32, it's not actually necessary that users access that entire data.frame rather than just either the matrices or the metadata

iainmstott added a commit to iainmstott/Rcompadre that referenced this issue Nov 21, 2018
Throughout, moved towards using methods to access data rather than through the slots directly.
Incorporated code from @patrickbarks regarding subset, replace, merge
@patrickbarks
Copy link
Collaborator

I really liked data and version for the slot names. They're short and simple and don't require any thought about case style (compadreData or CompadreData?), which is nice when I want to access the slots directly. And other packages use a slot called data for metadata-type data frames, so there's some standardization.

Anyone opposed to me reverting the slot names to data and version, which will be accessible with the accessors CompadreData() and CompadreVersion(), respectively? I think it's fine if the slot names don't exactly match the accessor names. They'd still be very similar, of course.

@patrickbarks
Copy link
Collaborator

closed by #43

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

No branches or pull requests

3 participants