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

Feature Request: Store Row Names in exprs slot of cytoframe #354

Open
DillonHammill opened this issue Dec 6, 2020 · 6 comments
Open

Feature Request: Store Row Names in exprs slot of cytoframe #354

DillonHammill opened this issue Dec 6, 2020 · 6 comments

Comments

@DillonHammill
Copy link
Contributor

Hi cytoverse team,

I was wondering if it would be possible to store information in the rownames of the exprs slot of a cytoframe. I am working on refining the plotting behavior within cyto_plot() and I would like to ensure that the same events are plotted when populations are overlaid. In order to do this I will need to reference each row based on its position within the root node (i.e. assign an index to each row within exprs(cf) ideally in rownames(exprs(cf))). I need to store this information at the cytoframe/cytoset level. I can easily write a wrapper for this using gh/gs_pop_get_data() and gh_pop_get_indices() to assign the indices to each row in each cytoframe but this does not get updated:

mt <- matrix(1:1000, ncol = 10, dimnames = list(NULL, LETTERS[1:10]))

# empty rownames
rownames(mt)

# flowFrame
fr <- flowFrame(mt)

# empty rownames
rownames(exprs(fr))

# replace rownames
rownames(exprs(fr)) <- paste0("A", "-", 1:100)

# rownames remain empty
rownames(exprs(fr))

This is because there is currently no replacement method for rownames in flowCore. Is this deliberate or would it be possible to add support for this?

Thanks for your help!
Dillon

@mikejiang
Copy link
Member

check out the latest cytolib/flowWorkspace, which has this feature implemented. see example https://rpubs.com/rglab/646420

BTW, if you are on mac or windows, cytoinstaller will help you install the more up-to-date binary packages than bioc devel.

@DillonHammill
Copy link
Contributor Author

Fantastic! Thanks so much @mikejiang! This will make my life a lot easier. I will update the CytoExploreR docs to use cytoinstaller, it makes it a lot easier for users to correctly install the cytoverse dependencies.

One last thing, do you think it would be possible to make gs_pop_get_data() retain these indices in the rownames by default? I will write a wrapper for CytoExploreR and cyto_plot() will then rely on this information being present. It would be great if this was implemented in flowWorkspace directly so users can use those APIs as well. No worries if you think this isn't worthwhile on your end, I will just have to tell users to stick to the CytoExploreR wrapper when extracting data from a GatingSet.

@mikejiang
Copy link
Member

gs_pop_get_data just grabs the reference of the underlying cytoframe. And currrently rownames is stored at cytoframe as the extra string attribute and once attached it uses the extra space and stays there unless you explicitly deleteit, and we don't actually want to duplicate the indices as rownames by default because indices is already stored as bool vector separately (more efficiently) somewhere else at GatingSet

@DillonHammill
Copy link
Contributor Author

Thanks @mikejiang. Perhaps, rather than attaching this at the level of gs_pop_get_data() I will instead annotate the cytoframes with indices as rownames prior to adding to GatingSet. That way when the populations are extracted downstream with gs_pop_get_data() the indices should retained without the need for a separate wrapper function.

Out of curiosity, is there a shortcut to install all packages with cytoinstaller? I think it is good practice for users to update all packages and it would be nice if there was an easy way to do this (perhaps if pkg isn't supplied to install_cyto?). It would be nice if there was a menu like the one when packages require updating that allows the user to indicate which packages to install (i.e. individual packages or all of them).

Thanks again for your help!

@DillonHammill
Copy link
Contributor Author

Thanks @mikejiang, happy to close this now and I will implement this on my end.

@DillonHammill
Copy link
Contributor Author

@mikejiang, this works well, but since I still have to go through a flowFrame intermediate when coercing a cytoset to a cytoframe I am losing my rownames. Has there been any progress on a direct cytoset to cytoframe coercion method - @jacobpwagner did take a look at this a while back?

cf <- as(cs, "cytoframe")

Current workaround involves coercion to flowFrame first and then conversion into a cytoframe:

fr <- as(cs, "flowFrame")  # rownames are lost here
cf <- flowFrame_to_cytoframe(fr)

@DillonHammill DillonHammill reopened this Dec 9, 2020
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