remove flag registry

[orbeckst]

tl;dr: Get rid of MDAnalysis.core.flags and replace with the lightest solution possible

The flag registry in MDAnalysis.core.flags (in MDAnalysis/core/__init__.py) determines global behavior. In principle, these flags can be changed by the user but in practice this seems to be rarely done. In any case, changes are not persistent and have to be changed for a new python session manually by setting the flag.

Action: remove MDAnalysis.core.flags

As discussed in #315 we decided that the flag registry in the current form does not serve a useful purpose and is too complicated for what it accomplishes. It should be reduced to the smallest and simplest data structure that is necessary, such as a simple dictionary or variables at the module level.

In particular there is currently no expectation that the user should be able to change any parameters in MDAnalysis that change the global behavior (such as changing the base units) because it adds additional assumptions to code and reduces portability.

Impact

Flags are used throughout the code to set defaults, e.g., in the readers and writers and some of the selections. All of this has to be changed.

Issues are marked with label Component-Configuration. In particular

• Periodic/Nonperiodic keywords to selections #759 Periodic/Nonperiodic keywords to selections
• Managing default selection keywords #104 Managing default selection keywords
• Use physical units in AtomGroup objects. #596 Using physical units in AtomGroups

[richardjgowers]

Hopefully we can just replace where flags offered options with a kwarg to the function

[orbeckst]

Yes, I hope so, too, but I didn't want to be overly prescriptive without properly reviewing the code in detail.

[jbarnoud]

Do we want to go through a transition phase with deprecation warnings? Or do we want to brutally remove the flags?

I would go for the deprecation warnings. For a version or two, the flag replacement could be mirrors to the flags by default. The flags mechanism could then remain available, but with a deprecation warning. I do not know if these flags are actually used; I never used them, but they control some critical mechanisms such as using periodic boundary conditions or not.

[orbeckst]

On 18 Mar, 2016, at 17:16, Jonathan Barnoud wrote:

Do we want to go through a transition phase with deprecation warnings? Or do we want to brutally remove the flags?

I would go for the deprecation warnings. For a version or two, the flag replacement could be mirrors to the flags by default. The flags mechanism could then remain available, but with a deprecation warning.

If it's not difficult then slap on these deprecation warnings as soon as you can. If you get them into 0.15.0 then we can drop them in 0.16.0....

I do not know if these flags are actually used; I never used them, but they control some critical mechanisms such as using periodic boundary conditions or not.

Yes, that was just a very, very horrible idea then. It makes it difficult to get reproducible behavior; I just hope most people never found out about it. Much better to use something like #759 .

[richardjgowers]

So here's a list of existing flags

• use_periodic_selections
• use_KDTree_routines
• convert_lengths
• length_unit
• time_unit
• speed_unit
• force_unit
• charge_unit
• use_pbc

We could add these all as kwargs to Universe init and make them boolean properties of Universe to recreate the behaviour.

So

u = mda.Universe(top, trj, length_unit='mile')

# OR

u.length_unit = 'mile'

# OR maybe

u.settings.length_unit = 'mile'

These would allow you to define the default behaviour.

[richardjgowers]

I would like to get this done in 0.16.0 (so we don't have to write flags into the new system only to kill them), so maybe we need to push out some more deprecations in a 0.15.1 once we've decided what the new form is.

[orbeckst]

My take on the current flags:

Flag	default	comment
use_periodic_selections	True	should just be a kwarg for select_atoms()
use_KDTree_routines	"fast"	should be kwarg for select_atoms()
convert_lengths	True	kwarg for readers – actually already implemented as kwarg convert_units
length_unit	"Angstrom"	not needed
time_unit	"ps"	not needed
speed_unit	"Angstrom/ps"	not needed
force_unit	"'kJ/(mol*Angstrom)'"	not needed
charge_unit	"e"	not needed
use_pbc	False	kwarg for all methods that work with distances (select_atoms() but also center_of_geometry() ...)

Any not needed should be effectively fixed at the current default.

EDITS:

• convert_units as kwarg remove flag registry #782 (comment)

[orbeckst]

I would not deprecate. Instead, add a fake core.flags object that raises an exception with a proper error message if someone tries to set a flag.

[jbarnoud]

I did use the convert_units argument of the XTC reader for some fancy purpose. I see how some people could have used the convert_lengths flags. In the "comment" column, I would have "should be kwarg for the readers".

[orbeckst]

Thanks, I updated the list.

[kain88-de]

I'm moving this to another milestone. 0.16.0 has enough breaking changes as it is.

[richardjgowers]

So for this to be completed, we need to add the kwargs for KDTree and periodic to select_atoms

[jbarnoud]

If I follow well, select_atoms needs a use_pbc and a use_KDTree_routines keyword arguments. The signature of select_atoms will then be: select_atoms(sel, updating=False, use_pbc=True, use_KDTree_routines='fast', *othersel, **selgroups).

The easiest way I see to implement these keywords would be to add them in core.selection.Selection. The current signature for a selection is Selection(parser, token), it would then be Selection(parser, token, use_pbc, use_KDTree_routines). Or more generically Selection(parser, token, **kwargs). I think it would be easy and would work, but I do not like much to pass around keyword arguments that are only relevant for a few cases.

I can implement this solution; but I would like to hear if you have better implementation ideas.

[jbarnoud]

What does remain to be done here after #2056?

[jbarnoud]

Sorry I have my own answer: actually removing the registry. I should not ask question before finishing my coffee.