Skip to content

Update indexing in Charges and Masses get_residues#2991

Merged
orbeckst merged 3 commits intodevelopfrom
issue2990-deprecated_array_indexing
Oct 16, 2020
Merged

Update indexing in Charges and Masses get_residues#2991
orbeckst merged 3 commits intodevelopfrom
issue2990-deprecated_array_indexing

Conversation

@jbarnoud
Copy link
Contributor

@jbarnoud jbarnoud commented Oct 16, 2020

Fixes #2990

Avoid indexing a numpy array with a list of arrays, use a tuple of array
instead. Multidimensional indexing of arrays with a non-tuple argument
is deprecated.

PR Checklist

  • Tests?
  • [ ] Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

Fixes #2990

Avoid indexing a numpy array with a list of arrays, use a tuple of array
instead. Multidimensional indexing of arrays with a non-tuple argument
is deprecated.
@pep8speaks
Copy link

pep8speaks commented Oct 16, 2020

Hello @jbarnoud! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 121:80: E501 line too long (81 > 79 characters)
Line 143:80: E501 line too long (81 > 79 characters)

Comment last updated at 2020-10-16 09:31:12 UTC

@codecov
Copy link

codecov bot commented Oct 16, 2020

Codecov Report

Merging #2991 into develop will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2991   +/-   ##
========================================
  Coverage    93.05%   93.05%           
========================================
  Files          186      186           
  Lines        24609    24609           
  Branches      3187     3187           
========================================
  Hits         22900    22900           
  Misses        1661     1661           
  Partials        48       48           
Impacted Files Coverage Δ
package/MDAnalysis/core/topologyattrs.py 96.49% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64a7c05...3b8fbd8. Read the comment docs.

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems non-controversial and easy to review: LGTM — and good idea to add the tests that now fail on the warning to avoid regression. We should follow the same pattern for future warning deprecation fixes (if we're not already doing it).

@orbeckst orbeckst merged commit 26880f0 into develop Oct 16, 2020
@orbeckst orbeckst deleted the issue2990-deprecated_array_indexing branch October 16, 2020 23:06
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this pull request Mar 30, 2021
* Fixes MDAnalysis#2990
* Update indexing in Charges and Masses get_residues: 
   Avoid indexing a numpy array with a list of arrays, use a tuple of array
   instead. Multidimensional indexing of arrays with a non-tuple argument
   is deprecated.
* Turn warnings into errors to avoid regression over MDAnalysis#2990
* Update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Residue.mass and Residue.charge use deprecated numpy indexing

4 participants