Closed jnothman closed 9 years ago
Thanks for this list. I'll have a look into this soon, if nobody else tackles it first
There are quite a bit of these to go through. I started with metrics and a couple in bayes.py. I'll do more later when I have time.
I had problems with the script using the newest code (iter_modules gone from "testing" for example). Also found another problem where six.py was referring to winregs (which doesn't work on Linux).
Another question, should I just wait until I've done all I'm going to do before sending a pull request, or send for each additional set I do?
Feel free to send PRs with partial fixes.
In sklearn,decomposition.MiniBatchSparsePCA, the residuals are never returned in the dictionnaryonline function. The documentations wrongly states that it has a method called error, giving the error at each iteration, but it does not exist.
See https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/decomposition/dict_learning.py line 643 and https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/decomposition/sparse_pca.py line 210
indeed
option 1: fix the docstring (easy) option 2: fix the code (harder)
I can do the easy one :)
Well, is the residual on the minibatch easily computable as the residuals for the fullbatch version? I though adding the option to return the residuals, then computing them for each batches and averaging the result at each iterations would be a way to do it.
Of course it's much easier to clean the doc, but adding more functionnality is probably better for the long run.
Well, is the residual on the minibatch easily computable as the residuals for the fullbatch version? I though adding the option to return the residuals, then computing them for each batches and averaging the result at each iterations would be a way to do it.
Of course it's much easier to clean the doc, but adding more functionnality is probably better for the long run.
indeed. PR welcome :)
I am working on this... Seems like there are still 80 120 odd such discrepancies to be fixed...
Should we close this as #4023 got merged?
I think we've not fixed cython level discrepancies....
I'm happy with this being closed. It would be nice to be able to configure landscape to check for any new errors of this type introduced by a PR, but I'm not sure if that configurability is available.
On 30 March 2015 at 08:07, ragv notifications@github.com wrote:
I think we've not fixed cython level discrepancies....
— Reply to this email directly or view it on GitHub https://github.com/scikit-learn/scikit-learn/issues/2062#issuecomment-87477532 .
having an automated check seems out of reach for the moment. I think we can close and come back to it in the future.
The parameters listed in class, method and function docstrings sometimes are obsolete, misspelled, or missing from the actual call signature. Often the docstring parameter should be removed, but in other cases like #2060 an argument should be added to the function.
Here are some possible discrepancies (including a number of false positives; false negatives include all Cython code) produced by this script: