Move box.Box to common.Box

[Eric Harper]

A while ago the redundancy of box being in its own module was discussed, and until recently, there was no module for "common" functions. Now that there is, I propose that we move box to common (and leave the box module as backwards compatible for a while)

[Eric Harper]

@joaander @csadorf @mspells @newmanr thoughts?

[Joshua Anderson]

freud is still version 0.x, API breaking changes are OK to make if they make sense. I haven't reviewed the freud API well enough recently to have any real input on whether box.box -> common.box makes sense.

[Eric Harper]

@joaander I'm not worried about breaking API, but I'd also rather not have a bunch of people complain that I break their scripts every 3 months (although IIRC the move to box.Box was last summer, so 9 months is fine), and just for good coding practice I started adding in import warnings for features that will be deprecated.

As far as the reasoning, with Simon and Matthew's help, I added in a feature which now auto-casts input arrays to contiguous arrays of the required dtype e.g. take a view of a numpy array of integers and cast to a float32 array. This was the first feature we added which is truly "common" i.e. used in every other function call in freud, so we created a common module to house it. Now that "common" exists, we should start considering which parts of freud are "common" and think about moving them there. Box is the first and most obvious of these.

[Matthew Spellings]

This sounds fine to me. IMO the "a while" unit of time that we would leave the box module as-is should be at least a year or so; little changes like this are trivial to fix (and the adapter to keep a freud.box.Box is virtually free), but annoying to encounter when people are just trying to get science done.

Personally, I would even be fine with exporting the box as freud.Box. It wouldn't ever conflict with freud submodules since we name those lower-case, and then users wouldn't have to worry about what module it is in, which sort of makes sense because it is used pretty much universally across freud.

[Carl Simon Adorf]

Following Matt's argument I'd vote against relocating the definition. I think that having the Box class in its own module is perfectly fine. I'd agree with Matt that pulling the class additionally into the root namespace is perfectly justifiable.

Following the convention of the Python standard library and popular packages, users should hardly ever need to dive into deeper namespaces unless it becomes enormous. Hoomd is a good example where this is necessary.

[Richmond Newman]

I think I'd be okay with it in the root namespace so long as there isn't interest in other possible boxes besides the provided triclinic implementation. The main arguments I'd have against moving it though is 1) Having to do something that takes time from fixing something else, and 2) Having its own module vaguely future proofs it from some farfetched scenarios in which people may want other box types, and most of freud only interacts with boxes via the wrap command. Here's at least two entirely speculative scenarios that might draw interest to other boxes:

• For some particularly wrap-heavy classes like the PMFT there might be some performance benefit to a simpler orthorhombic box.

• I can also vaguely conceive of some future odd cases where someone wants to wrap dimensions of other space-filling polyhedra that aren't representable by a triclinic box. Maybe this will arise in some study of lattice fluctuations where you need the crystal to be exactly commensurate with the box, and to be periodic on some set of lattice planes. I recall Jim at least needing to use parallelograms instead of rectangles for the phonon/libron study so that the box and crystal had same lattice vectors [Note: My recollection may be mistaken]. A three dimensional equivalent may not be workable with our current box. This would also be future proofing for something we can't yet really simulate, but probably could be be within the scope of a student research project to at least make a single-GPU or CPU simulation of such.

[Carl Simon Adorf]

With respect to Richmond's comment, I think that this is slightly different issue, but it can be easily solved by renaming the current class from Box to TriclinicBox and make Box inherit from TriclinicBox. The assumption underlying the current API is that we usually work with triclinic boxes.

We could implement different box types if needed later, the methods operating on those would then need to use that special box type.

Generally, I don't see any issue with pulling both the "default" box class and the specialized ones in to the root namespace.

With respect to the issue raised here (#142), the possibility of different box types actually strengthens the argument to keep all box definitions within the semantic namespace box rather than common in my opinion.

[Vyas Ramasubramani]

We have no plans to move Box to a different namespace.