davidcsterratt / retistruct

Computational reconstruction and transformation of flattened retinae
http://davidcsterratt.github.io/retistruct/
7 stars 7 forks source link

GUI seems to change R locale without notice #40

Closed jlause closed 4 years ago

jlause commented 5 years ago

Hey,

I am working with a fork of an older version of retistruct (0.5.12) on Ubuntu 16.04 / R 3.4.1. My goal is to add some small file saving functions to integrate it into our lab's workflow. During debugging, I noted that several of the locale variables (as queried by sessionInfo() or Sys.getlocale() were not set to their R default C, but to de_DE.UTF-8.

I read in the R documentation, that setting LC_NUMERIC to something else than C can cause unexpected behavior. In my case, it created weird problems writing *.csv files (see here for details), and setting LC_NUMERIC back to C solved those problems.

As I could not find out where exactly and why the change from C to de_DE.UTF-8 happens, I was a bit worried that this might cause other, maybe more subtle side effects on the processing done by retistruct.

Do you know anything about this?

One thing I already tested was that my R correctly uses the C locale on startup. Also, I could call some exposed functions from the retistruct package, and during these function calls, the locale was still C (I checked by printing the locale at various points with message(Sys.getlocale())). But when starting retistruct as GUI, the locale was no longer set to C.

The R manual linked above also mentions that usually there should be a warning when LC_NUMERIC is changed from C to anything else, but I never saw such a warning...

Looking forward to your feedback, and thanks a lot in advance :)

davidcsterratt commented 5 years ago

I've just checked (with R 3.6.1, Ubuntu 18.04 and Retistruct 0.6.0) and not been able to reproduce the problem. I would suggest trying with a more recent version of Ubuntu and R. Also, I would suggest using the v0.6.0 version of Retistruct for development, as it's structured using R6 classes, and will be used for future development. That said, I still need to document the R6 classes properly...

jlause commented 5 years ago

Great, thanks for the quick check! So the LC_NUMERIC did not change at all on your setup? Then it's high time for me to update ;)

I've considered switching to R 3.6.1 / retistruct 0.6.0 before, but seeing issue #27 I thought it would be good to wait until this is resolved, as I want to work with the saved reconstruction data.

Do you think this will happen soon?

davidcsterratt commented 5 years ago

I certainly want it to happen soon, and I've started work on it. However, I don't think it's going to happen before the end of the month.

In v0.5.x, I just saved the S3 reconstruction object using save(), and then loaded it using load. The R6 classes now used also contain member functions, and so doing the same thing would bring back the versions of the functions at the time of saving. Therefore I want to strip out the functions before saving. On loading this object, i would then instantiate an empty object, and then copy the list members over. It's a bit fiddly more fiddly than that, but it should be doable.

Another idea I had was to change the format of the saved file from rData to HDF5, which could then be imported more easily into Matlab or python. Would that be of interest? It would also require the function-stripping code.

bc commented 5 years ago

+1 for rData output with embedded functions

HDF5 is a nice to have in the short term. If we want to make Retistruct an open access REST API then HDF5 or JSON would allow better flexibility and smaller installation overhead.

MSR


From: David C Sterratt notifications@github.com Sent: Thursday, August 22, 2019 6:57:41 AM To: davidcsterratt/retistruct retistruct@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [davidcsterratt/retistruct] GUI seems to change R locale without notice (#40)

I certainly want it to happen soon, and I've started work on it. However, I don't think it's going to happen before the end of the month.

In v0.5.x, I just saved the S3 reconstruction object using save(), and then loaded it using load. The R6 classes now used also contain member functions, and so doing the same thing would bring back the versions of the functions at the time of saving. Therefore I want to strip out the functions before saving. On loading this object, i would then instantiate an empty object, and then copy the list members over. It's a bit fiddly more fiddly than that, but it should be doable.

Another idea I had was to change the format of the saved file from rData to HDF5, which could then be imported more easily into Matlab or python. Would that be of interest? It would also require the function-stripping code.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_davidcsterratt_retistruct_issues_40-3Femail-5Fsource-3Dnotifications-26email-5Ftoken-3DADJCPNRPEJVMDZDMNPHO2QLQF2LNLA5CNFSM4INBO3PKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD45FVQY-23issuecomment-2D523918019&d=DwMCaQ&c=clK7kQUTWtAVEOVIgvi0NU5BOUHhpN0H8p7CSfnc_gI&r=6fqZQUOBmd_2gsqT3P9R0ZoczL8LM-a9V4ueaZ430nw&m=Sw9kfQ21Q5iv-P30JV_Ev6vIQVgPFlltGDxSOUPmJck&s=oeAg54goeA26jrXOVEovCjFAzzr0kg-AYOt0JRyVICg&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ADJCPNR4X45UIW5MCXTTXY3QF2LNLANCNFSM4INBO3PA&d=DwMCaQ&c=clK7kQUTWtAVEOVIgvi0NU5BOUHhpN0H8p7CSfnc_gI&r=6fqZQUOBmd_2gsqT3P9R0ZoczL8LM-a9V4ueaZ430nw&m=Sw9kfQ21Q5iv-P30JV_Ev6vIQVgPFlltGDxSOUPmJck&s=v2d--QcLJwUXjecl-99-Bgp2aQxKuvULh36dt_fVAcc&e=.

jlause commented 5 years ago

I certainly want it to happen soon, and I've started work on it. However, I don't think it's going to happen before the end of the month.

Cool! Looking forward!

Another idea I had was to change the format of the saved file from rData to HDF5, which could then be imported more easily into Matlab or python. Would that be of interest? It would also require the function-stripping code.

Having HDF5 instead of (or in addition to) rData would be great I think!

davidcsterratt commented 4 years ago

Issue #27 is now fixed and I have created issue #44 to discuss HDF5 output.