search

qiime2 / q2-diversity

BSD 3-Clause "New" or "Revised" License
4 stars 45 forks source link

API `n_jobs` to `thread` breaking change in #281 #293

Closed gwarmstrong closed 4 years ago

gwarmstrong commented 4 years ago

Problem Description PR #281 changed the parameter n_jobs to threads in beta_phylogenetic this is an API breaking change, and is also inconsistent with beta. https://github.com/qiime2/q2-diversity/pull/281/files#diff-2da5bf068d9eb56eb1591bb87cfef6b9L82 https://github.com/qiime2/q2-diversity/pull/281/files#diff-574e13c8cab0e3a31785d39784248c22R12-R19

Encountered this when the CI for https://github.com/knightlab-analyses/regression-benchmarking failed.

Questions

  1. Can n_jobs be retained for backward compatibility? It seems like the description was changed to more accurately reflect what is happening in q2-diversity-lib/unifrac. But that aspect is a bit technical and is covered by the descriptions, so I don't think this necessarily requires a parameter name change?
thermokarst commented 4 years ago

We've been cleaning up a bunch of API stuff in this plugin in the 2020.6 and 2020.8 cycles. I'm open to changing it back to n_jobs, but at the same time, this is basically what our release schedule is for, so that we can introduce breaking changes when necessary. I'm just getting ready to start the 2020.8 release, BTW.

thermokarst commented 4 years ago

and is also inconsistent with beta.

Forgot to mention, the difference is deliberate, see https://github.com/qiime2/q2-diversity/issues/214 for more details.

gwarmstrong commented 4 years ago

Thanks for the link! I'll take that discussion as the official ruling, then.