mdanalysis icon indicating copy to clipboard operation
mdanalysis copied to clipboard

Fix: Align.py doctest error

Open HeetVekariya opened this issue 2 years ago • 10 comments

Partially address #3925

Changes made in this Pull Request:

  • Doctest for align.py (package/MDAnalysis/analysis/align.py) file contains error because wildcard match is not correctly implemented.
  • Removed > from the Expected output such that error can be solved.

PR Checklist

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

Developers certificate of origin


📚 Documentation preview 📚: https://mdanalysis--4373.org.readthedocs.build/en/4373/

HeetVekariya avatar Dec 19 '23 02:12 HeetVekariya

I will update the change log when reviewed by maintainers, such that unnecessary merge conflicts can be avoided.

HeetVekariya avatar Dec 19 '23 02:12 HeetVekariya

Linter Bot Results:

Hi @HeetVekariya! Thanks for making this PR. We linted your code and found the following:

Some issues were found with the formatting of your code.

Code Location Outcome
main package ⚠️ Possible failure
testsuite ✅ Passed

Please have a look at the darker-main-code and darker-test-code steps here for more details: https://github.com/MDAnalysis/mdanalysis/actions/runs/9726622099/job/26845277057


Please note: The black linter is purely informational, you can safely ignore these outcomes if there are no flake8 failures!

github-actions[bot] avatar Dec 19 '23 03:12 github-actions[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.41%. Comparing base (b808ef1) to head (9f7a747). Report is 55 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff             @@
##           develop    #4373     +/-   ##
==========================================
  Coverage    93.41%   93.41%             
==========================================
  Files          171      185     +14     
  Lines        22512    23626   +1114     
  Branches      4129     4129             
==========================================
+ Hits         21029    22070   +1041     
- Misses         963     1036     +73     
  Partials       520      520             

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Dec 19 '23 03:12 codecov[bot]

@HeetVekariya CI is still showing errors with analysis/align.py, could you please look into it?

I'd also ask to change the "Fixes https://github.com/MDAnalysis/mdanalysis/issues/3925" into something else like "Partially address #3925", otherwise the issue will be automatically closed when we merge this PR.

RMeli avatar Dec 19 '23 22:12 RMeli

  • @RMeli can you show me which type of error you got ?
  • Because for me it passes all the test cases.
/mdanalysis/package/MDAnalysis$ pytest -v --disable-pytest-warnings --doctest-modules analysis/align.py 
============================================================= test session starts ==============================================================
platform linux -- Python 3.10.13, pytest-7.4.3, pluggy-1.3.0 -- /home/heet/anaconda3/envs/mdanalysis-dev/bin/python
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase(PosixPath('/home/heet/Work/Development/mdanalysis/package/MDAnalysis/.hypothesis/examples'))
rootdir: /home/heet/Work/Development/mdanalysis/package
plugins: anyio-4.0.0, hypothesis-6.92.1
collected 2 items                                                                                                                              

analysis/align.py::MDAnalysis.analysis.align PASSED                                                                                      [ 50%]
analysis/align.py::MDAnalysis.analysis.align.rotation_matrix PASSED                                                                      [100%]

======================================================= 2 passed, 23 warnings in 12.43s ========================================================

HeetVekariya avatar Dec 20 '23 01:12 HeetVekariya

@HeetVekariya I'm referring to the errors in the doctest CI.

RMeli avatar Dec 21 '23 08:12 RMeli

  • @RMeli from the logs i found that error is generated due to this 1 and 2.
  • But for my local set up i does not gives error.
  • So should i give it a chance by changing the code to this:

For 1:

>>> ref = mda.Universe(PDB_small)
>>> mobile = mda.Universe(PSF, DCD)
>>> rmsd(mobile.select_atoms('name CA').positions, ref.select_atoms('name CA').positions, center=True)
21.892591663632704

For 2:

>>> ref = mda.Universe(PDB_small)
>>> mobile = mda.Universe(PSF, DCD)
>>> rmsd(mobile.select_atoms('name CA').positions, ref.select_atoms('name CA').positions, 
...      superposition=True)
6.809396586471815

HeetVekariya avatar Dec 21 '23 10:12 HeetVekariya

Thanks for looking into it. I'm not very well versed in doctests for MDAnalysis, I'll need a bit of time to understand why your setup is different from what we have in CI.

RMeli avatar Dec 21 '23 15:12 RMeli

@RMeli are you able to do a review here or should I try to find someone else?

orbeckst avatar Mar 30 '24 17:03 orbeckst

Sorry, I completely missed this. =/ I'm cycling the PR so we can have access to the logs.

RMeli avatar Jun 29 '24 20:06 RMeli