Add AtomGroup stub, fixes #1067 #1070

kain88-de · 2016-11-08T18:51:52Z

Changes made in this Pull Request:

create atomgroup stub

Any class used from this module will raise a warning. The classes work
without needed to be specially imported. This should work best for other other.

I would also like that a warning is raised when someone calls from MDAnalysis.core import AtomGroup but I think that might be impossible.

PR Checklist

Tests?
~~[ ] Docs?~~
CHANGELOG updated?
Issue raised/referenced?

kain88-de · 2016-11-08T21:40:06Z

package/MDAnalysis/core/AtomGroup.py

+Universe = deprecate_class(universe.Universe,
+                           "MDAnalysis.core.AtomGroup.Universe has been removed."
+                           "Please use MDAnalaysis.core.universe.Universe."
+                           "This stub will be removed in 1.0")


Any suggestions if the message is OK?

richardjgowers · 2016-11-08T21:42:18Z

I've been thinking we should move common things to the top namespace, so
mda.Universe, mda.AtomGroup etc.

On Tue, 8 Nov 2016, 9:40 p.m. Max Linke, [email protected] wrote:

@kain88-de commented on this pull request.

In package/MDAnalysis/core/AtomGroup.py
#1070 (review)
:
+from . import universe
+
+
+def deprecate_class(class_new, message):
"""utility to deprecate a class"""

class new_class(class_new):
   def **init**(self, _args, *_kwargs):
       super(new_class, self).**init**(_args, *_kwargs)
       warnings.warn(message, DeprecationWarning)
return new_class
+Universe = deprecate_class(universe.Universe,
                      "MDAnalysis.core.AtomGroup.Universe has been removed."
                      "Please use MDAnalaysis.core.universe.Universe."
                      "This stub will be removed in 1.0")
Any suggestions if the message is OK?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1070 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI0jBx6GmASeT6VUIihlBXtRF40gPy8tks5q8Ow2gaJpZM4Ksxvd
.

kain88-de · 2016-11-08T21:45:34Z

I've been thinking we should move common things to the top namespace, so mda.Universe, mda.AtomGroup etc.

How would that help with the deprecation warnings I introduced here? If one uses mda.Universe everything is fine and should throw any warnings.

richardjgowers · 2016-11-08T21:51:54Z

For the error messages, you should just recommend the top namespace

On Tue, 8 Nov 2016, 9:45 p.m. Max Linke, [email protected] wrote:

I've been thinking we should move common things to the top namespace, so
mda.Universe, mda.AtomGroup etc.

How would that help with the deprecation warnings I introduced here? If
one uses mda.Universe everything is fine and should throw any warnings.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#1070 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI0jB_pozeg55mOMNF87r1rOuo1NvWK5ks5q8O1_gaJpZM4Ksxvd
.

kain88-de · 2016-11-08T21:59:04Z

Good idea I can do that for the universe. For the others we don't have top name space imports. So I would leave the others as they are right now.

kain88-de · 2016-11-09T10:12:52Z

testsuite/MDAnalysisTests/core/test_atomgroup.py

+def test_usage_warning():
+    with warnings.catch_warnings(record=True) as warn:
+        warnings.simplefilter('always')
+        mda.core.AtomGroup.Universe(PSF, DCD)


How can I initialize the other classes to trigger a warning?

kain88-de · 2016-11-09T10:13:50Z

Would using a naked AtomGroup still work like it did before 0.16.0?

richardjgowers · 2016-11-09T10:18:06Z

The new signature for Group is Group(indices, universe), eg AtomGroup(np.array([0, 4, 5]), universe)

kain88-de · 2016-11-09T10:25:14Z

How would I initialize an Atomgroup in 0.15.0? My current solution won't work at all if I can't make the classes in mda.core.AtomGroup being called the same way as in 0.15.0

richardjgowers · 2016-11-09T10:36:06Z

It would have been AtomGroup(list of atoms). If we want perfect compatibility, we could make the AtomGroup in core.AtomGroup work like the old one but raise a warning about the new one. Or alternatively, have 2 branches in AtomGroup.__init__ that figures out how it's been called and makes it work either way (raising a warning if it's the "old" way).

kain88-de · 2016-11-11T16:10:50Z

@richardjgowers Thanks for the changes. I'll do some final clean up on the weekend.

richardjgowers · 2016-11-11T16:12:57Z

Yeah I can't see a way for (single) Atom, Res and Seg to work, so they
might just have to fail with a nice message.

On Fri, 11 Nov 2016, 4:10 p.m. Max Linke, [email protected] wrote:

@richardjgowers https://github.com/richardjgowers Thanks for the
changes. I'll do some final clean up on the weekend.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1070 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI0jBxRVmF6ZMHmPbqTEoJK4zZUyH-qzks5q9JOLgaJpZM4Ksxvd
.

kain88-de · 2016-11-11T16:24:14Z

Atom, Res and Seg to work

Wouldn't mda.core.AtomGroup(u, index)[0] work? Trying it in the terminal suggests that it would.

Any class used from this module will raise a warning. The classes work without needed to be specially imported.

kain88-de · 2016-11-13T19:24:55Z

@richardjgowers I see now what you mean with the Atom/Residue/Segment classes. I don't think there is a nice way to enable the use. I don't even know how to add a nice error message that catches all relevant cases.

So assuming that no one used single Atoms a lot I would leave it as is now. The most important classes are covered.

* Add AtomGroup stub Any class used from this module will raise a warning. The classes work without needed to be specially imported. * fix Universe Warning Message * update changelog * add test for deprecation warning * Added compatibility for old AtomGroup init methods * Added tests for deprecated Group init methods

* Completes #1070 and moves us towards v1.0 #2443 * Removed AtomGroup stubs. * Removed unnecessary import from groups.py * Removes deprecation test * Update CHANGELOG

* Completes MDAnalysis#1070 and moves us towards v1.0 MDAnalysis#2443 * Removed AtomGroup stubs. * Removed unnecessary import from groups.py * Removes deprecation test * Update CHANGELOG

kain88-de commented Nov 8, 2016

View reviewed changes

kain88-de commented Nov 9, 2016

View reviewed changes

kain88-de and others added 6 commits November 13, 2016 19:54

Add AtomGroup stub

69c1310

Any class used from this module will raise a warning. The classes work without needed to be specially imported.

fix Universe Warning Message

8090950

update changelog

1e67900

add test for deprecation warning

07ff321

Added compatibility for old AtomGroup init methods

32594dc

Added tests for deprecated Group init methods

a7860bc

kain88-de force-pushed the atomgroup-stub branch from 0e9b036 to a7860bc Compare November 13, 2016 19:22

richardjgowers approved these changes Nov 13, 2016

View reviewed changes

richardjgowers merged commit 7cb2431 into MDAnalysis:develop Nov 13, 2016

richardjgowers self-assigned this Nov 13, 2016

kain88-de mentioned this pull request Nov 13, 2016

Update mda dependency datreant/MDSynthesis#63

Merged

kain88-de deleted the atomgroup-stub branch November 18, 2016 07:33

IAlibay mentioned this pull request Feb 28, 2020

Atomgroup removal #2562

Merged

4 tasks

orbeckst pushed a commit that referenced this pull request Feb 29, 2020

Atomgroup removal (#2562)

964d40e

* Completes #1070 and moves us towards v1.0 #2443 * Removed AtomGroup stubs. * Removed unnecessary import from groups.py * Removes deprecation test * Update CHANGELOG

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AtomGroup stub, fixes #1067 #1070

Add AtomGroup stub, fixes #1067 #1070

kain88-de commented Nov 8, 2016 •

edited

Loading

kain88-de Nov 8, 2016

richardjgowers commented Nov 8, 2016

@kain88-de commented on this pull request.

kain88-de commented Nov 8, 2016

richardjgowers commented Nov 8, 2016

kain88-de commented Nov 8, 2016

kain88-de Nov 9, 2016

kain88-de commented Nov 9, 2016

richardjgowers commented Nov 9, 2016

kain88-de commented Nov 9, 2016

richardjgowers commented Nov 9, 2016

kain88-de commented Nov 11, 2016

richardjgowers commented Nov 11, 2016

kain88-de commented Nov 11, 2016

kain88-de commented Nov 13, 2016

Add AtomGroup stub, fixes #1067 #1070

Add AtomGroup stub, fixes #1067 #1070

Conversation

kain88-de commented Nov 8, 2016 • edited Loading

PR Checklist

kain88-de Nov 8, 2016

Choose a reason for hiding this comment

richardjgowers commented Nov 8, 2016

@kain88-de commented on this pull request.

kain88-de commented Nov 8, 2016

richardjgowers commented Nov 8, 2016

kain88-de commented Nov 8, 2016

kain88-de Nov 9, 2016

Choose a reason for hiding this comment

kain88-de commented Nov 9, 2016

richardjgowers commented Nov 9, 2016

kain88-de commented Nov 9, 2016

richardjgowers commented Nov 9, 2016

kain88-de commented Nov 11, 2016

richardjgowers commented Nov 11, 2016

kain88-de commented Nov 11, 2016

kain88-de commented Nov 13, 2016

kain88-de commented Nov 8, 2016 •

edited

Loading