replace atoms.coordinates() with atoms.positions

[orbeckst]

Since 2.0, atoms does not have coordinates() anymore and only positions is supported.

• Needs to be fixed:
  

  mdanalysis/package/MDAnalysis/analysis/encore/covariance.py

  Line 218 in 46c8bad

  reference_coordinates = reference_atom_selection.atoms.coordinates()

• Also, fix benchmarks that still use coordinates() (but this could be made another PR)

[orbeckst]

Output from git grep 'coordinates():

benchmarks/benchmarks/analysis/rms.py:        # ag.coordinates()
benchmarks/benchmarks/analysis/rms.py:            self.A = self.u.atoms.coordinates().copy()[:num_atoms]
benchmarks/benchmarks/analysis/rms.py:            self.B = self.u.atoms.coordinates().copy()[:num_atoms]
package/CHANGELOG:    'get_positions()' can be used instead of the 'coordinates()'
package/MDAnalysis/analysis/encore/covariance.py:        reference_coordinates = reference_atom_selection.atoms.coordinates()
package/MDAnalysis/coordinates/PDB.py:        self._check_pdb_coordinates()
testsuite/MDAnalysisTests/coordinates/test_timestep_api.py:            self.Timestep.from_coordinates()

[Atharva7K]

Hey, I'm very new to open source and would like to try my hands on this, so any pointers for this fix would be highly appreciated. For the first fix, I just have to replace the function calls for coordinates() by get_positions()? Or there's more to it than this?

[orbeckst]

Hello @Atharva7K , you'd replace .coordinates() with .positions. If this is in code that wasn't tested before then we'd want to add a test exercising this code but we can give guidance on how to do this.

[Atharva7K]

Thank you for replying @orbeckst . Apparently, rms.py has already been tested so I can directly do the replacement there. But for covariance.py, I will require guidance to add the tests as you said so please advice me on how to get started. Should I first create a PR fixing the issue in rms.py and create a separate PR for adding the tests?

[manishsaini6421]

I am outreachy applicant and want to do this. I am new in opensource so can you guide me please how can i do this?
And How to start contributing?

[orbeckst]

@manishsaini6421 , the User Guide has a section Contributing Code to the Main Codebase. In short, you need to learn how to use git and GitHub. You'd fork the MDAnalysis repository. Then you fix the code in your fork of MDAnalysis (in your own account). Then you create a pull request (PR) against the main MDAnalysis repository. Then we review your PR and discuss it with you. Once everything looks good (typically after multiple rounds of discussions and improvements) we merge your code into the main repository.

[orbeckst]

In order to work on this issue, you need to make a PR that references this issue number. We don't have people "reserve" issues to work on, we just look at the first work that comes in.

[manishsaini6421]

@orbeckst Thanks for detailed discription i'll do it now.

[manishsaini6421]

@orbeckst Sir, i have fixed this issue, can you check it once please.

[orbeckst]

@manishsaini6421 I can see that you created PR #3576. — very good. One of our core developers already reviewed your PR so your next step is to address their comments.

[manishsaini6421]

@orbeckst I have made some changes but still 2 test cases fails , what should i do now?

[Davichet-e]

Shouldn't this issue be closed? I think it was fixed by this commit: 51661637ee1444a15eec331e8777cbc60c9c9836^^

[orbeckst]

It was actually fixed in PR #3621 as a side-effect — we should have added it to CHANGELOG, I suppose.

[orbeckst]

I am going to raise a separate issue for the benchmarks.