search

lhenneman / hyspdisp

4 stars 4 forks source link

setwd() inside functions #37

Closed schoolAccountMajaG closed 5 years ago

schoolAccountMajaG commented 5 years ago

This is related to issue https://github.com/lhenneman/hyspdisp/issues/18.

The functions should not change directories as is being done here: https://github.com/garbulinskamaja/hyspdisp/blob/master/R/hyspdisp_fac_model_parallel.R#L140-L141

If the structure is defined properly we can adjust the code and this is not needed.

lhenneman commented 5 years ago

To do this (and I think it's a good idea), we need to change SplitR here: https://github.com/lhenneman/SplitR/blob/3f055a6a3909c2c803848af80940f79f5f3408a9/R/hysplit_dispersion.R#L800

schoolAccountMajaG commented 5 years ago

@cchoirat Is this a good idea to change SplitR?

lhenneman commented 5 years ago

We already use a modified version of SplitR called here: https://github.com/lhenneman/hyspdisp/blob/687bc2927358fb2fa117252e07b760afdff96bdb/DESCRIPTION#L17

We needed to change a few things (e.g., being able to define met_dir and npart).

On Fri, Jun 14, 2019 at 10:06 AM Maja notifications@github.com wrote:

@cchoirat https://github.com/cchoirat Is this a good idea to change SplitR?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/lhenneman/hyspdisp/issues/37?email_source=notifications&email_token=AHMZYW5PVCDGQ4VFH2UYOB3P2OQWXA5CNFSM4HYGJP5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXW4RJY#issuecomment-502122663, or mute the thread https://github.com/notifications/unsubscribe-auth/AHMZYW64K4QYXTKBT2FARPLP2OQWXANCNFSM4HYGJP5A .

-- Lucas R.F. Henneman, Ph.D. (404) 788-2161 lhenneman@gmail.com

cchoirat commented 5 years ago

We can work on the fork that @lhenneman mentions, but get (all) ready for a possible cascade of unexpected events.

schoolAccountMajaG commented 5 years ago

Is copying the function from the original SplitR, renaming it, changing slightly, and putting in hyspdisp an option? With credit to SplitR, of course.

cchoirat commented 5 years ago

you don't like the fork idea?

On Fri, Jun 14, 2019 at 4:32 PM Maja notifications@github.com wrote:

Is copying the function from the original SplitR, renaming it, changing slightly, and putting in hyspdisp an option? With credit to SplitR of course.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lhenneman/hyspdisp/issues/37?email_source=notifications&email_token=AA73AZN5ALVEPWFQH6KJSBDP2OTYLA5CNFSM4HYGJP5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXW65OQ#issuecomment-502132410, or mute the thread https://github.com/notifications/unsubscribe-auth/AA73AZLIYVPGSAZBZ3NJP4DP2OTYLANCNFSM4HYGJP5A .

schoolAccountMajaG commented 5 years ago

I don't like a possible cascade of unexpected events.

cchoirat commented 5 years ago

might be the same with just a function

On Fri, Jun 14, 2019 at 4:34 PM Maja notifications@github.com wrote:

I don't like a possible cascade of unexpected events.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lhenneman/hyspdisp/issues/37?email_source=notifications&email_token=AA73AZIRQHJJUPQWMGWZ6ZDP2OT73A5CNFSM4HYGJP5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXW7CZI#issuecomment-502133093, or mute the thread https://github.com/notifications/unsubscribe-auth/AA73AZOLYVKA4ID6EIWP2EDP2OT73ANCNFSM4HYGJP5A .

schoolAccountMajaG commented 5 years ago

new directory set up structure has been implemented in disperseR