-
-
Notifications
You must be signed in to change notification settings - Fork 26k
Release of version 0.19.1 #9607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Is it controversial to include |
I'm fine with including it. It could be considered a bugfix. |
Fine with me. You may want to add #9649 when/if it gets merged. |
Probably this would be good to add #9683, so that we ensure that 0.19.1 will pass the tests with the next numpy release. |
I think this would be great to add #9670 which makes the nose dependency optional. |
@lesteve for the benefit of contrib developers?
…On 12 September 2017 at 19:31, Loïc Estève ***@***.***> wrote:
I think this would be great to add #9670
<#9670> which makes the
nose dependency optional.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#9607 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz63B3MpIjkKtjI4sDQ06TtPXbjbQiks5shk9agaJpZM4O_VYV>
.
|
Done |
aa1dcb9
to
07e0a77
Compare
sweet :) |
Circle fails?
|
Oh is that the google stock retry thing? |
It is, though the retry is included in 0.19.1.... |
07e0a77
to
7d6f307
Compare
I pushed a fix for numpy dev (test only) and the whole Google finance commit saga, hoping that it will make Circle pass. |
And of course, now the conda update saga strikes again ... |
the pleasures of CI.... |
Thanks for adding #9623! About whats_new, I was thinking, given the reorganisation in separate files in master, that it is simpler to add the whats_new entries in this PR (rather than in each separate PR), and once 0.19.1 is released to update v0.19.rst in master. |
Yes :)
|
Btw, @lesteve, one approach is to release this, then merge the entire branch into master, just to be sure nothing is missed from master. |
Codecov Report
@@ Coverage Diff @@
## 0.19.X #9607 +/- ##
==========================================
+ Coverage 96.17% 96.17% +<.01%
==========================================
Files 335 335
Lines 61873 62117 +244
==========================================
+ Hits 59504 59744 +240
- Misses 2369 2373 +4
Continue to review full report at Codecov.
|
#9932 is missing |
merged #9593 |
(not cherry picked, though. Can you cherry pick, I'll try to check the PR). |
Ready to merge and upload when you are, @amueller. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP8 error for your reference, also, Reiichiro Nakano (#9593) seems missing from contributors.
@@ -16,12 +16,15 @@ | |||
from sklearn.utils.testing import assert_almost_equal | |||
from sklearn.utils.testing import assert_raises | |||
from sklearn.utils.testing import assert_raise_message | |||
from sklearn.utils.testing import assert_warns_message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicate import, remove either one
from sklearn.utils.testing import assert_raises_regex | ||
from sklearn.utils.testing import assert_greater | ||
from sklearn.utils.testing import assert_less | ||
from sklearn.utils.testing import assert_array_almost_equal | ||
from sklearn.utils.testing import assert_array_equal | ||
from sklearn.utils.testing import assert_warns | ||
from sklearn.utils.testing import assert_warns_message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicate import, remove either one
Thank you!! |
Ok I'll look and merge and release tomorrow. Deal? |
I pushed some more documentation fixes to your branch. let me know if you disagree with any of them. Apart from what I mentioned above, it looks pretty complete. I'll double check tomorrow that there's nothing in there that we don't want. |
The doc fixes are fine. I'm ambivalent about fixes to minor longstanding
bugs like that to affinity propagation. including them in a minor release
is more overhead than benefit.
|
merge at will. |
ok cool! I think I agree with your assessment about the other bug fixes. |
hm contributions are sorted by commits not alphabetically, though it looks like we did the same for 0.19. I kind preferred alphabetically but I don't mind enough to change it ;) |
hm pulling ndcg and dcg is a breaking change between 0.19 and 0.19.1, right? That doesn't seem very nice? Shouldn't we deprecate and warn? |
Lol have to merge manually to avoid squashing. I'm doing that now and tagging. I'd appreciate a summary of why removing these scores is ok, but I expect you and @agramfort thought this through. |
builds are starting... working on conda-forge now |
hm my internet is kinda dead. if anyone wants to do conda-forge, please go ahead. |
Not sure if this is right conda-forge/scikit-learn-feedstock#60 |
also see ContinuumIO/anaconda-issues#6809 |
thanks. ndcg was too broken for anyone to have successfully used it
|
and I'd never noticed what's new was previously alphabetical!
|
When we removed the commit counts I wanted to do alphabetical. Need to push the wheels to pypi, rest looks done. Will to tomorrow morning. |
Oh, you mean that order. Ah. Right.
|
Hmmm it looks like the nose hard dependency removal is not part of 0.19.1 😕 ... I am pretty sure I checked at one point and #9607 (comment) seems to say the same thing. It's weird also I am not able to see the diff or commit of this PR ... |
Sorry. I'm not sure how that happened. Well, I guess we'd better document it in what's new for 0.20. |
Oh well ... maybe something to keep in mind if there is a 0.19.2. |
Should the associated milestone be closed? |
Thanks, I closed the 0.19.1 milestone. |
For merging to 0.19.X upon release.