{"action":"opened","issue":{"url":"https://api.github.com/repos/PecanProject/pecan/issues/801","repository_url":"https://api.github.com/repos/PecanProject/pecan","labels_url":"https://api.github.com/repos/PecanProject/pecan/issues/801/labels{/name}","comments_url":"https://api.github.com/repos/PecanProject/pecan/issues/801/comments","events_url":"https://api.github.com/repos/PecanProject/pecan/issues/801/events","html_url":"https://github.com/PecanProject/pecan/issues/801","id":150153700,"number":801,"title":"incorrect identification of leap years; multiple implementations. ","user":{"login":"dlebauer","id":464871,"avatar_url":"https://avatars.githubusercontent.com/u/464871?v=3","gravatar_id":"","url":"https://api.github.com/users/dlebauer","html_url":"https://github.com/dlebauer","followers_url":"https://api.github.com/users/dlebauer/followers","following_url":"https://api.github.com/users/dlebauer/following{/other_user}","gists_url":"https://api.github.com/users/dlebauer/gists{/gist_id}","starred_url":"https://api.github.com/users/dlebauer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dlebauer/subscriptions","organizations_url":"https://api.github.com/users/dlebauer/orgs","repos_url":"https://api.github.com/users/dlebauer/repos","events_url":"https://api.github.com/users/dlebauer/events{/privacy}","received_events_url":"https://api.github.com/users/dlebauer/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/PecanProject/pecan/labels/bug","name":"bug","color":"e11d21"},{"url":"https://api.github.com/repos/PecanProject/pecan/labels/Documentation","name":"Documentation","color":"5319e7"},{"url":"https://api.github.com/repos/PecanProject/pecan/labels/enhancement","name":"enhancement","color":"5319e7"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-04-21T18:26:41Z","updated_at":"2016-04-21T18:26:41Z","closed_at":null,"body":"The logic to test if a year is a leap year has been implemented perhaps a dozen times independently [within the PEcAn code base](https://github.com/PecanProject/pecan/search?utf8=%E2%9C%93&q=leap), including once as the function [`is.leap`](https://github.com/PecanProject/pecan/blob/1743b942fdba4adb7cd6cc7716f2f3ae258eb2c0/models/ed/inst/timeutils.R).\r\n2. there are [many cases where `year %% 4 i== 0`](https://github.com/PecanProject/pecan/search?utf8=%E2%9C%93&q=%22year+%25%25+4%22&type=Code) is used although it should be `year %% 400 == 0 | (year %% 4 == 0 & year %% 100 != 0)` (`is.leap` and `leap_year` implement this correctly).\r\n1. The lubridate package has a function `leap_year` that returns T/F. We already use this function [in many places](https://github.com/PecanProject/pecan/search?utf8=%E2%9C%93&q=%22leap_year%22&type=Code) and I suggest that this is a good solution\r\n3. (just a comment / food for thought) testing if a year is a leap year is often followed by calculating time steps (e.g. days or seconds/year). `lubridate` also provides convenience functions for this that would be less error prone than declaring seconds / year or days / month constants repeatedly in the code.\r\n\r\nTODO\r\n\r\n* [ ] (bugfix) All of the invalid uses of `year %% 4 i== 0` should be replaced by a valid function (`leap_year`)\r\n* [ ] (enhancement) deprecate `is.leap` and replace it with `lubridate::leap_year`\r\n* [ ] (documentation) add some notes about existing 'helper' functions to the model template README"}}