Skip to content

Adding elements attribute to PDB parser and unit tests#2648

Closed
AmeyaHarmalkar wants to merge 1 commit intoMDAnalysis:developfrom
AmeyaHarmalkar:pdb_element_attrs
Closed

Adding elements attribute to PDB parser and unit tests#2648
AmeyaHarmalkar wants to merge 1 commit intoMDAnalysis:developfrom
AmeyaHarmalkar:pdb_element_attrs

Conversation

@AmeyaHarmalkar
Copy link
Contributor

Fixes #2647

Changes made in this Pull Request:

PR Checklist

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

@codecov
Copy link

codecov bot commented Mar 26, 2020

Codecov Report

Merging #2648 into develop will increase coverage by 0.39%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2648      +/-   ##
===========================================
+ Coverage    90.61%   91.01%   +0.39%     
===========================================
  Files          174      174              
  Lines        23554    23607      +53     
  Branches      3072     3086      +14     
===========================================
+ Hits         21343    21485     +142     
+ Misses        1585     1498      -87     
+ Partials       626      624       -2     
Impacted Files Coverage Δ
package/MDAnalysis/topology/PDBParser.py 99.44% <100.00%> (+0.03%) ⬆️
package/MDAnalysis/lib/log.py 95.78% <0.00%> (-1.92%) ⬇️
package/MDAnalysis/analysis/pca.py 95.29% <0.00%> (-0.27%) ⬇️
...sis/analysis/encore/clustering/ClusteringMethod.py 96.92% <0.00%> (-0.18%) ⬇️
...age/MDAnalysis/analysis/hbonds/hbond_autocorrel.py 87.34% <0.00%> (-0.16%) ⬇️
package/MDAnalysis/lib/transformations.py 78.47% <0.00%> (-0.15%) ⬇️
package/MDAnalysis/analysis/waterdynamics.py 90.51% <0.00%> (-0.14%) ⬇️
...ckage/MDAnalysis/analysis/hbonds/hbond_analysis.py 81.49% <0.00%> (-0.11%) ⬇️
package/MDAnalysis/analysis/helanal.py 87.93% <0.00%> (-0.07%) ⬇️
package/MDAnalysis/analysis/base.py 100.00% <0.00%> (ø)
... and 9 more

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 e416aa7...ec0b2f5. Read the comment docs.

@AmeyaHarmalkar
Copy link
Contributor Author

@IAlibay the new PR for the issue like you suggested.
Let me know if I need to do any further changes.

@IAlibay
Copy link
Member

IAlibay commented Mar 27, 2020

@AmeyaHarmalkar I think there has been some misunderstanding, I meant that you should finish PR #2627 solely allowing for elements to be parsed if they are all present & normal (as you are doing here). Then open up a new PR/issue set for dealing with alternative cases where elements are non-normal, including any appropriate changes to guessers.py.

@AmeyaHarmalkar
Copy link
Contributor Author

Oh, I completely misunderstood you then. Should I do a hard reset on the other branch then do the changes? I will close this PR and delete the issue.

@IAlibay
Copy link
Member

IAlibay commented Mar 27, 2020

Oh, I completely misunderstood you then. Should I do a hard reset on the other branch then do the changes? I will close this PR and delete the issue.

Sorry about that, this is definitely on me for bad communications. Probably closing this PR would be a good idea, however you can keep the issue open, just add it to the header of #2627 and we can close it when we merge the PR.

Re: hard reset -> just revert the one file, you can do that using git checkout: https://stackoverflow.com/questions/215718/how-can-i-reset-or-revert-a-file-to-a-specific-revision

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PDB Parsers should provide an elements attribute

3 participants