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

Export DFrame() as primary generator instead of DataFrame()? #90

Closed
mjsteinbaugh opened this issue Oct 14, 2021 · 4 comments
Closed

Export DFrame() as primary generator instead of DataFrame()? #90

mjsteinbaugh opened this issue Oct 14, 2021 · 4 comments

Comments

@mjsteinbaugh
Copy link

This is a relatively minor issue, but what about exporting DFrame() as the primary generator for DFrame objects, now that DataFrame has been changed to a virtual class?

See also for DFrame/DataFrame class change in 2019:
https://www.bioconductor.org/help/course-materials/2019/BiocDevelForum/02-DataFrame.pdf

This would better match the conventions defined in other Bioconductor packages, such as GenomicRanges::GRanges().

@hpages
Copy link
Contributor

hpages commented Oct 14, 2021

Maybe but note that the other convention (i.e. constructor function named like the virtual class) is also widely in used e.g. GenomicRanges::GRangesList(), GenomicRanges::GPos(), IRanges::IntegerList(), etc...

@mjsteinbaugh
Copy link
Author

Yeah that's a good point. I think there's still confusion about how DFrame/DataFrame works after the changes a couple of years ago.

For reference, here's what @hpages is referring to regarding the example virtual classes mentioned above:

class(GenomicRanges::GRangesList())
## [1] "CompressedGRangesList"
## attr(,"package")
## [1] "GenomicRanges"

class(GenomicRanges::GPos())
## [1] "UnstitchedGPos"
## attr(,"package")
## [1] "GenomicRanges"

class(IRanges::IntegerList())
## [1] "CompressedIntegerList"
## attr(,"package")
## [1] "IRanges"

@hpages
Copy link
Contributor

hpages commented Oct 14, 2021

I think there's still confusion about how DFrame/DataFrame works after the changes a couple of years ago

From an end-user point of view nothing should have changed. They still construct a DataFrame with the DataFrame() constructor function and the returned object is still displayed as being a DataFrame. At this point, since DFrame is still the only concrete DataFrame subclass, I still consider it an implementation detail and the less the end user knows about it the better. It would not be desirable to see people start to code specifically towards DFrame (e.g. by using things like is(x, "DFrame") and/or by documenting the argument of their function as being a DFrame) when they don't need to.

@mjsteinbaugh
Copy link
Author

It would not be desirable to see people start to code specifically towards DFrame

OK that was the main point I was wondering about. I won't do this in any of my packages.

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

2 participants