Alias bfactor to tempfactor #3345

lilyminium · 2021-05-31T23:58:58Z

Fixes #1901

Changes made in this Pull Request:

Aliases bfactor to tempfactor ~~with some abstract-ish machinery (aliases) in case we do this again in the future.~~
Added deprecation warning on access

I forget, did we want to warn people that this attribute now has 2 names?

I kept most of the mentions of bfactor in the tests the same to make sure we're not altering behaviour.

PR Checklist

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

pep8speaks · 2021-05-31T23:59:03Z

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

In the file package/MDAnalysis/topology/__init__.py:

Line 161:80: E501 line too long (91 > 79 characters)

Comment last updated at 2021-08-01 17:02:38 UTC

package/MDAnalysis/core/topologyattrs.py

package/CHANGELOG

package/MDAnalysis/core/topologyattrs.py

codecov · 2021-06-01T01:07:59Z

Codecov Report

Merging #3345 (b82ba6e) into develop (1e31180) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #3345   +/-   ##
========================================
  Coverage    93.62%   93.62%           
========================================
  Files          176      176           
  Lines        22835    22848   +13     
  Branches      3224     3224           
========================================
+ Hits         21379    21392   +13     
  Misses        1405     1405           
  Partials        51       51

Impacted Files	Coverage Δ
package/MDAnalysis/converters/RDKit.py	`98.06% <ø> (-0.02%)`	⬇️
package/MDAnalysis/topology/PDBParser.py	`100.00% <ø> (ø)`
package/MDAnalysis/topology/__init__.py	`100.00% <ø> (ø)`
package/MDAnalysis/core/topologyattrs.py	`96.77% <100.00%> (+0.05%)`	⬆️
package/MDAnalysis/core/universe.py	`97.98% <100.00%> (-0.02%)`	⬇️
package/MDAnalysis/topology/MMTFParser.py	`98.07% <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 1e31180...b82ba6e. Read the comment docs.

orbeckst · 2021-06-02T20:27:22Z

I am a bit busy today and might not get to it immediately, sorry.

…

On Jun 1, 2021, at 6:22 PM, Lily Wang ***@***.***> wrote: @lilyminium <https://github.com/lilyminium> requested your review on: #3345 <#3345> Alias bfactor to tempfactor.

lilyminium · 2021-06-02T20:30:56Z

No rush at all, take your time :)

…

On 2 Jun 2021, at 1:27 pm, Oliver Beckstein ***@***.***> wrote: I am a bit busy today and might not get to it immediately, sorry. > On Jun 1, 2021, at 6:22 PM, Lily Wang ***@***.***> wrote: > > > @lilyminium <https://github.com/lilyminium> requested your review on: #3345 <#3345> Alias bfactor to tempfactor. > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3345 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHNMOXMT2BI766NTM4HHXRDTQ2H3RANCNFSM453T5CEA>.

IAlibay

Couple of small comments.

package/CHANGELOG

IAlibay · 2021-06-02T20:43:20Z

package/MDAnalysis/core/topologyattrs.py

@@ -265,6 +265,25 @@ def _attach_transplant_stubs(attribute_name, topology_attribute_class):
                setattr(dest_class, method_name, stub)


+# TODO: remove bfactors in 3.0
+BFACTOR_WARNING = ("The bfactor topology attribute is only "


This should be documented somewhere.

I'm not sure we really document all the topology attributes anywhere. I added it as a docstring to the bfactor/s properties?

Edit: and I'll update the user guide

Yeah that sounds reasonable, for some reason I thought we had a big table somewhere, but maybe I'm getting confused with something else.

in __init__.py, but I think that might firstly be woefully out of date, and secondly never mentioned bfactors to start off with :p

testsuite/MDAnalysisTests/core/test_universe.py

IAlibay

sorry it took so long to re-review.

Couple of questions and some docs changes for pdb/mmtf

IAlibay · 2021-06-24T18:15:33Z

package/MDAnalysis/core/topologyattrs.py

@@ -265,6 +265,25 @@ def _attach_transplant_stubs(attribute_name, topology_attribute_class):
                setattr(dest_class, method_name, stub)


+# TODO: remove bfactors in 3.0
+BFACTOR_WARNING = ("The bfactor topology attribute is only "


Yeah that sounds reasonable, for some reason I thought we had a big table somewhere, but maybe I'm getting confused with something else.

package/MDAnalysis/core/topologyattrs.py

package/MDAnalysis/topology/MMTFParser.py

package/MDAnalysis/topology/PDBParser.py

testsuite/MDAnalysisTests/core/test_copying.py

IAlibay · 2021-07-19T12:46:12Z

@lilyminium if you can give a yea/nay to my review comments we can aim to merge this.

IAlibay

lgtm - please do merge whenever

orbeckst

minor doc issues that should be corrected, please

package/MDAnalysis/core/topologyattrs.py

package/MDAnalysis/topology/PDBParser.py

Co-authored-by: Irfan Alibay <[email protected]>

IAlibay · 2021-08-12T16:39:03Z

@orbeckst is this good to merge now?

IAlibay · 2021-08-12T19:28:30Z

🎉 one step closer to a 2.0 release!

IAlibay reviewed Jun 1, 2021

View reviewed changes

package/MDAnalysis/core/topologyattrs.py Show resolved Hide resolved

lilyminium force-pushed the alias-bfactor branch from 0441262 to 8d57949 Compare June 1, 2021 00:19

IAlibay reviewed Jun 1, 2021

View reviewed changes

package/CHANGELOG Outdated Show resolved Hide resolved

package/MDAnalysis/core/topologyattrs.py Outdated Show resolved Hide resolved

lilyminium force-pushed the alias-bfactor branch 2 times, most recently from 766929f to b8e7f7c Compare June 1, 2021 23:19

lilyminium requested review from orbeckst and IAlibay June 2, 2021 01:22

IAlibay requested changes Jun 2, 2021

View reviewed changes

lilyminium mentioned this pull request Jun 3, 2021

Bfactors now an alias of tempfactors MDAnalysis/UserGuide#148

Open

jbarnoud added this to the 2.0 milestone Jun 5, 2021

IAlibay requested changes Jun 24, 2021

View reviewed changes

richardjgowers approved these changes Jun 28, 2021

View reviewed changes

lilyminium force-pushed the alias-bfactor branch 2 times, most recently from 173f09c to 89072af Compare July 19, 2021 13:14

IAlibay approved these changes Jul 28, 2021

View reviewed changes

orbeckst requested changes Jul 28, 2021

View reviewed changes

package/MDAnalysis/core/topologyattrs.py Show resolved Hide resolved

package/MDAnalysis/core/topologyattrs.py Show resolved Hide resolved

package/MDAnalysis/topology/PDBParser.py Outdated Show resolved Hide resolved

lilyminium and others added 9 commits August 1, 2021 09:57

aliased bfactor

ab4f5bc

pep8

cf668e7

loop over groups

a076d1d

Update package/MDAnalysis/core/topologyattrs.py

352fd25

Co-authored-by: Irfan Alibay <[email protected]>

Update package/CHANGELOG

6b9cab2

Co-authored-by: Irfan Alibay <[email protected]>

rm rdkit failing test

ea87acc

add warnings to everything

c84094c

suggestions from review

a00b62c

Update MMTFParser.py

2de32c2

lilyminium added 2 commits August 1, 2021 09:57

update versionchanged

fdc10c9

pep8 and docs changes

b82ba6e

lilyminium force-pushed the alias-bfactor branch from 89072af to b82ba6e Compare August 1, 2021 17:02

lilyminium requested a review from orbeckst August 1, 2021 23:35

orbeckst approved these changes Aug 12, 2021

View reviewed changes

orbeckst assigned IAlibay Aug 12, 2021

IAlibay merged commit 09f50ec into MDAnalysis:develop Aug 12, 2021

IAlibay added the maintainability label Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alias bfactor to tempfactor #3345

Alias bfactor to tempfactor #3345

lilyminium commented May 31, 2021 •

edited

Loading

pep8speaks commented May 31, 2021 •

edited

Loading

codecov bot commented Jun 1, 2021 •

edited

Loading

orbeckst commented Jun 2, 2021 via email

lilyminium commented Jun 2, 2021 via email

IAlibay left a comment

IAlibay Jun 2, 2021

lilyminium Jun 3, 2021 •

edited

Loading

IAlibay Jun 24, 2021

lilyminium Jul 19, 2021

IAlibay left a comment

IAlibay Jun 24, 2021

IAlibay commented Jul 19, 2021

IAlibay left a comment

orbeckst left a comment

IAlibay commented Aug 12, 2021

IAlibay commented Aug 12, 2021

Alias bfactor to tempfactor #3345

Alias bfactor to tempfactor #3345

Conversation

lilyminium commented May 31, 2021 • edited Loading

PR Checklist

pep8speaks commented May 31, 2021 • edited Loading

Comment last updated at 2021-08-01 17:02:38 UTC

codecov bot commented Jun 1, 2021 • edited Loading

Codecov Report

orbeckst commented Jun 2, 2021 via email

lilyminium commented Jun 2, 2021 via email

IAlibay left a comment

Choose a reason for hiding this comment

IAlibay Jun 2, 2021

Choose a reason for hiding this comment

lilyminium Jun 3, 2021 • edited Loading

Choose a reason for hiding this comment

IAlibay Jun 24, 2021

Choose a reason for hiding this comment

lilyminium Jul 19, 2021

Choose a reason for hiding this comment

IAlibay left a comment

Choose a reason for hiding this comment

IAlibay Jun 24, 2021

Choose a reason for hiding this comment

IAlibay commented Jul 19, 2021

IAlibay left a comment

Choose a reason for hiding this comment

orbeckst left a comment

Choose a reason for hiding this comment

IAlibay commented Aug 12, 2021

IAlibay commented Aug 12, 2021

lilyminium commented May 31, 2021 •

edited

Loading

pep8speaks commented May 31, 2021 •

edited

Loading

codecov bot commented Jun 1, 2021 •

edited

Loading

lilyminium Jun 3, 2021 •

edited

Loading