Make DensityAnalysis use the Results class

[IAlibay]

Towards #3261

Changes made in this Pull Request:

• deprecate .density in favour of .results.density.

PR Checklist

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

[pep8speaks]

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

• In the file package/MDAnalysis/analysis/density.py:

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

Comment last updated at 2021-05-06 16:22:50 UTC

[codecov]

Codecov Report

Merging #3263 (7736cfa) into develop (b5bff33) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #3263   +/-   ##
========================================
  Coverage    93.02%   93.02%           
========================================
  Files          172      172           
  Lines        22694    22698    +4     
  Branches      3193     3193           
========================================
+ Hits         21110    21114    +4     
  Misses        1534     1534           
  Partials        50       50           

Impacted Files	Coverage Δ
package/MDAnalysis/analysis/density.py	82.45% <100.00%> (+0.42%)	⬆️

---

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 b5bff33...7736cfa. Read the comment docs.

[IAlibay]

forgot to update the docs properly -- in progress, also the deprecate decorator is just making the docs messy, so I'll just manually add the warning.

[PicoCentauri]

Sad the the docs become messy when we use mda's @deprecate decorator...

[IAlibay]

Yeah it enforces docstring which is just not very human friendly :(
For the sake of a couple of extra lines I'm happy just adding the messages by hand.

[PicoCentauri]

Yes, but this still looks good and as a good template for the other classes.

[orbeckst]

Looking good. Only required thing is another CHANGELOG entry under deprecations.

Thanks for creating a template for the other cases!

[orbeckst]

Please also resolve the conflicts. Thank you!!

[orbeckst]

Going forward, is there an improvement we could make to our deprecate decorator?

There is a little “deprecated” package https://python-deprecated.readthedocs.io/en/latest/api.html with a similar decorator. Maybe we check it out and adapt if it does anything better than ours (the source code is only a single file).

[IAlibay]

Going forward, is there an improvement we could make to our deprecate decorator?

There is a little “deprecated” package https://python-deprecated.readthedocs.io/en/latest/api.html with a similar decorator. Maybe we check it out and adapt if it does anything better than ours (the source code is only a single file).

From this PR, the main issue is that it didn't want to play well with not having a docstring (I don't remember this being a problem before, I think it might be a clash with @property?). The default message isn't super user friendly (at least in my opinion) either, but that is overridable via message (although at that point you've written as many lines as it would take to just write your own warning message..).

[orbeckst]

Maybe defining a doc string for the attribute first would help. But ultimately, just doing it explicitly is not wrong and if it's clearer then all the better.

Perhaps there's a clever helper that could do something like

@deprecated_results_attribute(doc="Density contains a :class:`Density` instance blabla")
def density(self):
    pass

which takes func.__name__ to index self.results and adds the docs with the deprecation.

(But time's short so simple & fast is a virtue...)