timjmiller / wham

State-space, age-structured fish stock assessment model
https://timjmiller.github.io/wham
Other
32 stars 16 forks source link

Small bug in N1_model=1 on R side #55

Closed Cole-Monnahan-NOAA closed 2 years ago

Cole-Monnahan-NOAA commented 2 years ago

There's a small typo that causes an error in the prepare_wham_input function. You can see it here:

https://github.com/timjmiller/wham/blob/master/R/set_NAA.R#L14

where the == turns it into a boolean (or maybe NULL?) which causes a downstream error. Needs to just be =. I was going to do a PR but apparently we have line returns configured differently. I'll need to configure locally to match your EOL settings on git before doing PRs like in your preferred workflow.

timjmiller commented 2 years ago

Thanks Cole for trying this option and finding the bug. I will get this fixed on devel branch. No need for you to change your settings.

On Feb 26, 2022, at 12:24 AM, Cole Monnahan @.***> wrote:



There's a small typo that causes an error in the prepare_wham_input function. You can see it here:

https://github.com/timjmiller/wham/blob/master/R/set_NAA.R#L14

where the == turns it into a boolean (or maybe NULL?) which causes a downstream error. Needs to just be =. I was going to do a PR but apparently we have line returns configured differently. I'll need to configure locally to match your EOL settings on git before doing PRs like in your preferred workflow.

— Reply to this email directly, view it on GitHub https://github.com/timjmiller/wham/issues/55, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEIGN7G4TSOOH3RSALHGWD3U5BPXJANCNFSM5PMBWTRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

jimianelli commented 2 years ago

re EOL "settings". Are we still at this point? Can't just abandon windows format altogether?

Most OS's now have dos2unix and the converse unix2dos (why!) which treats these. Seems .

some description: https://www.centennialsoftwaresolutions.com/post/lineendings

https://support.nesi.org.nz/hc/en-gb/articles/218032857-Converting-from-Windows-style-to-UNIX-style-line-endings

https://www.cs.toronto.edu/~krueger/csc209h/tut/line-endings.html https://www.quora.com/Although-DOS-is-obsolete-why-doesn ’t-Microsoft-ditch-it-and-build-a-Unix-based-OS

so seems silly to ever worry about EOL compatibility for dos machines...and am sure emacs probably has a fix for this?

fun!

On Mon, Feb 28, 2022 at 6:52 AM Tim Miller @.***> wrote:

Thanks Cole for trying this option and finding the bug. I will get this fixed on devel branch. No need for you to change your settings.

On Feb 26, 2022, at 12:24 AM, Cole Monnahan @.***> wrote:



There's a small typo that causes an error in the prepare_wham_input function. You can see it here:

https://github.com/timjmiller/wham/blob/master/R/set_NAA.R#L14

where the == turns it into a boolean (or maybe NULL?) which causes a downstream error. Needs to just be =. I was going to do a PR but apparently we have line returns configured differently. I'll need to configure locally to match your EOL settings on git before doing PRs like in your preferred workflow.

— Reply to this email directly, view it on GitHub https://github.com/timjmiller/wham/issues/55, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AEIGN7G4TSOOH3RSALHGWD3U5BPXJANCNFSM5PMBWTRQ

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/timjmiller/wham/issues/55#issuecomment-1054335758, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUW7YQKBXEVW4I6PSMAU2LU5OD4PANCNFSM5PMBWTRQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Jim Ianelli

timjmiller commented 2 years ago

Typo now fixed on devel branch