OpenTreeOfLife / reference-taxonomy

Open Tree Reference Taxonomy (OTT) tools
BSD 2-Clause "Simplified" License
11 stars 12 forks source link

Swap Proximity heuristic to fix #304; address OTT id problem found by @hyanwong #307

Closed jar398 closed 7 years ago

jar398 commented 7 years ago

Please merge the other PRs first.

This PR is ready to go, and it's an open question whether to update the writeup to reflect this new version instead of draft3 (or draft3a which is identical in its statistics). Draft3 is not suitable for production use and IMO shouldn't be the version that goes into Zenodo. This version (draft4) I expect to work fine in production, but it would be good to get some vetting from @mtholder and/or @hyanwong to be sure.

The fix for #304 was trivial, although investigating the impact of the fix took a while. The Zaglossus id fix took longer, as it was actually a combination of several bugs in id assignment code written since 2.10 was deployed.

kcranston commented 7 years ago

Will updating to draft4 only affect the results section (tables, and summary stats in the text)?

jar398 commented 7 years ago

draft5 now. Besides the table, summary stats, and the figure that gives alignment summary stats, there is just one small change, to the description of the Proximity heuristic.

@mtholder has agreed to test draft5. Writeup changes should wait for that.

kcranston commented 7 years ago

OK, I am convinced about updating. Thanks for the info.

On Sun, Feb 26, 2017, 13:44 Jonathan A Rees notifications@github.com wrote:

draft5 now. Besides the table, summary stats, and the figure that gives alignment summary stats, there is just one small change, to the description of the Proximity heuristic.

@mtholder https://github.com/mtholder has agreed to test draft5. Writeup changes should wait for that.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/OpenTreeOfLife/reference-taxonomy/pull/307#issuecomment-282577145, or mute the thread https://github.com/notifications/unsubscribe-auth/AATC4nyl_x1Of_BtEIxbukvxeWMHKOXwks5rgcgogaJpZM4MMO4p .

kcranston commented 7 years ago

Can you fix conflicts, @jar398 ?