imfeldn / swiss_pheno

Creative Commons Attribution 4.0 International
1 stars 1 forks source link

refactoring code + data shuffle + moving to RDS #2

Closed khufkens closed 1 year ago

khufkens commented 1 year ago

Hi @imfeldn,

These are a lot of changes, if you feel uncomfortable pulling this in at once, don't. Basically I couldn't test the new RDS conversion routines. It is just saving it in a different format and should be ok, but better measure twice and cut once.

I suggest to download (git clone) my repository first (so the one under khufkens), then formally test the code (especially the generation of the new RDS files) in a temporary location on your machine (you can quit after one file, doesn't need to be the whole routine obviously). If this all works, then pull this in!

khufkens commented 1 year ago

Could you hold off merging this, need to figure some things out first!!

khufkens commented 1 year ago

Thanks, will sync my fork if you push new changes. Thanks also for collaborating on github so smoothly!

imfeldn commented 1 year ago

Hi,

I pushed some changes. Now it should run based on the original data. I also added the "class feature" in the data selection/naming. I hope that's okay. It allows to easily add more of the pheno stations with lower quality rating.

Now the other scripts need updating too, since they still rely on the .RData objects. I can make these adjustments there, and also, e.g. move the ploting out of the scripts. But maybe it makes sense to discuss, where we're heading with respect to the calibration?

Cheers,

Noemi


Von: Koen Hufkens @.> Gesendet: Dienstag, 15. August 2023 18:57 An: imfeldn/swiss_pheno @.> Cc: Imfeld, Noemi (GIUB) @.>; State change @.> Betreff: Re: [imfeldn/swiss_pheno] refactoring code + data shuffle + moving to RDS (PR #2)

Sie erhalten nicht oft eine E-Mail von @.*** Erfahren Sie, warum dies wichtig isthttps://aka.ms/LearnAboutSenderIdentification

Thanks, will sync my fork if you push new changes. Thanks also for collaborating on github so smoothly!

— Reply to this email directly, view it on GitHubhttps://github.com/imfeldn/swiss_pheno/pull/2#issuecomment-1679293154, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AH67PVCOGKVTSIRTHSOKISLXVOS7ZANCNFSM6AAAAAA3RCLHHA. You are receiving this because you modified the open/close state.Message ID: @.***>

khufkens commented 1 year ago

Yes, I didn't know what the class feature was so I left as is. Good point including this if this is an important (flexible) variable. Anything that makes code more generalizable.

For consistency you can change everything from RData, but I didn't want to insist on this as this is mostly data preparation and this is basically out of reach of anyone who wants to reproduce the results (it is "given").

We should have a look at where we go with calibration. I'll open a separate discussion (issue) for this, and it might be good to sit down on a Tuesday. With the workflows in place it should be easier to juggle some of these things and coordinate better.

imfeldn commented 1 year ago

I'll update the other scripts then as well. Should we meet next Tuesday, 22 August to discuss the open issue?


Von: Koen Hufkens @.> Gesendet: Mittwoch, 16. August 2023 11:42 An: imfeldn/swiss_pheno @.> Cc: Imfeld, Noemi (GIUB) @.>; State change @.> Betreff: Re: [imfeldn/swiss_pheno] refactoring code + data shuffle + moving to RDS (PR #2)

Sie erhalten nicht oft eine E-Mail von @.*** Erfahren Sie, warum dies wichtig isthttps://aka.ms/LearnAboutSenderIdentification

Yes, I didn't know what the class feature was so I left as is. Good point including this if this is an important (flexible) variable. Anything that makes code more generalizable.

For consistency you can change everything from RData, but I didn't want to insist on this as this is mostly data preparation and this is basically out of reach of anyone who wants to reproduce the results (it is "given").

We should have a look at where we go with calibration. I'll open a separate discussion (issue) for this, and it might be good to sit down on a Tuesday. With the workflows in place it should be easier to juggle some of these things and coordinate better.

— Reply to this email directly, view it on GitHubhttps://github.com/imfeldn/swiss_pheno/pull/2#issuecomment-1680289942, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AH67PVEFT7Y2W4UGUBM4CHTXVSIXTANCNFSM6AAAAAA3RCLHHA. You are receiving this because you modified the open/close state.Message ID: @.***>

khufkens commented 1 year ago

Sure, does 15:30 work like before? I've opened a new issue #3 so if you have any ideas / remarks you can put it down there.

imfeldn commented 1 year ago

Perfect, Tuesday, 15.30 is fine!