Changes encore.hes() details output #2484
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2484 +/- ##
===========================================
+ Coverage 90.56% 90.68% +0.11%
===========================================
Files 170 170
Lines 22830 22828 -2
Branches 2944 2943 -1
===========================================
+ Hits 20677 20701 +24
+ Misses 1559 1540 -19
+ Partials 594 587 -7
Continue to review full report at Codecov.
|
|
@lilyminium would you mind having a quick look and comment if this PR fixes your original issue? Please feel free to ping me when you need a final review for merge. Thanks. |
|
Looks good to me, thanks @mtiberti! |
There was a problem hiding this comment.
@lilyminium thanks for the review.
@mtiberti can you please fix the documentation and CHANGELOG – see my comments. Thank you!
package/CHANGELOG
Outdated
| @@ -19,6 +19,7 @@ mm/dd/yy richardjgowers, kain88-de, lilyminium, p-j-smith, bdice, joaomcteixeira | |||
| * 0.21.0 | |||
|
|
|||
| Fixes | |||
| * encore.hes() returns non-array details (Issue #2465) | |||
There was a problem hiding this comment.
This is not a fix but a Change because for a user who wants to process the output something changes – please move to Change section.
There was a problem hiding this comment.
Also add to Changes that hes() does not take the details keyword argument anymore.
package/CHANGELOG
Outdated
| @@ -48,6 +49,7 @@ Fixes | |||
| * Added parmed to setup.py | |||
|
|
|||
| Enhancements | |||
| * encore.hes() always returns details, consistently with ces and dres (Issue #2465) | |||
There was a problem hiding this comment.
Are these really two different things that the PR does? To me it looks like a single entry under Changes.
|
Can you please also resolve the conflict on CHANGELOG so that it can be merged cleanly? |
|
sorry - I hadn't realised the build was failing because of wrong spacing in the docstring. It should be fixed now. |
|
@lilyminium , does this look good to you now? (I don't want to squash-merge without giving you a chance to look, given that @richardjgowers asked for your opinion.) |
| (array([[ 0. , 38279683.95892926], | ||
| [ 38279683.95892926, 0. ]]), None) | ||
| >>> HES, details = encore.hes([ens1, ens2]) | ||
| >>> print HES |
There was a problem hiding this comment.
Could you please just change this to print(HES) for python 3 users?
|
Many thanks @lilyminium |
|
Thank you both! |
Fixes #2465
Changes made in this Pull Request:
detailsflag. This is more consistent with the behaviour ofcesanddres. Previously,Nonewas returned instead if thedetailsflag wasFalse.PR Checklist