SofieVG / FlowSOM

Using self-organizing maps for visualization and interpretation of cytometry data
61 stars 26 forks source link

Fix creating scaled.scale and scaled.center #24

Closed exaexa closed 5 years ago

exaexa commented 5 years ago

Hello Sofie!

We've recently noticed that FlowSOM version 1.15.3 no longer creates the fsom$scaled.scale and fsom$scaled.center objects, which we used for several purposes (more precisely-- Abhishek ran into the issue around an hour ago while preparing some new poster). Quick search in the commit history showed that these might have been removed inadvertenly; this commit fixes that.

The other new case (when only a few of the columns is scaled) is handled as well; the code just puts identity scale/centering values into the scaled.* of columns that do not get scaled. I hope it works right with the rest of the workflow. The other possibility is to leave the scaling vectors sparse; let me know if I should change the commit that way.

If the scaled.scale and scaled.center removal was intentional, please let me know, I'd remove the dependency on that from the rest of our code.

For compatibility reasons I also increased the FlowSOM version by 0.0.1 in the same commit (so that packages may depend on this by using >=1.15.4 dep), but that is only cosmetic and not really required; feel free to discard that change.

Thank you in advance,

with best regards, -mk

(attached commit message follows)

R attr()s do not get preserved when assigning data into a sub-range of a larger
frame (even if the range is just "whole data"); for that reason attr(fsom$data,
"scaled:scale") is not created after a recent update of scale handing. This
attempts to reconstruct a reasonable scaled.scale/scaled.center in both new
cases.
SofieVG commented 5 years ago

Hi Mirek,

Thanks for notifying me about this issue! You are certainly right that I did not intend to remove the fsom$scaled.scale and fsom$scaled.center parameters, the goal is exactly that they will be present (e.g. to undo the scaling when necessary). However, I'm not sure your code is completely accurate? In the case where there are some scaling parameters passed along, the data does not get updated in your code? Or am I missing something?

Enjoy the weekend! Sofie

On Tue, 30 Apr 2019 at 18:50, Mirek Kratochvil notifications@github.com wrote:

Hello Sofie!

We've recently noticed that FlowSOM version 1.15.3 no longer creates the fsom$scaled.scale and fsom$scaled.center objects, which we used for several purposes (more precisely-- Abhishek ran into the issue around an hour ago while preparing some new poster). Quick search in the commit history showed that these might have been removed inadvertenly; this commit fixes that.

The other new case (when only a few of the columns is scaled) is handled as well; the code just puts identity scale/centering values into the scaled.* of columns that do not get scaled. I hope it works right with the rest of the workflow. The other possibility is to leave the scaling vectors sparse; let me know if I should change the commit that way.

If the scaled.scale and scaled.center removal was intentional, please let me know, I'd remove the dependency on that from the rest of our code.

For compatibility reasons I also increased the FlowSOM version by 0.0.1 in the same commit (so that packages may depend on this by using >=1.15.4 dep), but that is only cosmetic and not really required; feel free to discard that change.

Thank you in advance,

with best regards, -mk

(attached commit message follows)

R attr()s do not get preserved when assigning data into a sub-range of a larger frame (even if the range is just "whole data"); for that reason attr(fsom$data, "scaled:scale") is not created after a recent update of scale handing. This attempts to reconstruct a reasonable scaled.scale/scaled.center in both new cases.


You can view, comment on, or merge this pull request online at:

https://github.com/SofieVG/FlowSOM/pull/24 Commit Summary

  • Fix creating scaled.scale and scaled.center

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/SofieVG/FlowSOM/pull/24, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOS72YNSFYXM5RFBJNOP63PTB2DVANCNFSM4HJOMVYQ .

exaexa commented 5 years ago

However, I'm not sure your code is completely accurate?

Whoops, I somehow dropped out the important line while getting the commit ready. Sorry. :]

Also, after some peering at how others use the scaled.scale/scaled.center values, I changed the vectors back to the sparse form for simplicity. (From the interfacing point of view, filling in the identity values also kindof discarded the information about the exact user input, which was not entirely right.)

The commit should be fixed now (it's forcepushed over the original; I see GitHub handled that just right).

Thanks, -mk