If you set B wrt A and then B wrt C, then overwrite A wrt B, B wrt C shouldn't change. It's not clear that is what is happening here.

I think these tests would be much clearer if you assert against the entire _dcm_dict. After each call to orient* we know what the whole dictionary should look like, so test the whole dict not just specific entires.

Please also write some comments that explain what you are testing here. It's not clear without very careful look at all these sin's and cosines.

*Please use the new functions e.g. orient_axis in all future code.

If you set B wrt A and then B wrt C, then overwrite A wrt B, B wrt C shouldn't change. It's not clear that is what is happening here.

Yes this is happening here.

I'll update test cases to make it clearer.

The unit tests should also have some tests that ensure the difference in the _dcm_dict and _dcm_cache is handled properly. Adding those tests with this change, would ensure we are breaking anything regarding that difference.

Yes I will add seperate unit tests for _dcm_cache and _dcm_dict.

One or more tests like:

B.orient(A)
C.orient(B)
D.orient(C)
# check that all _dcm_dicts have all adjacent relations properly defined and check that _dcm_cache == _dcm_dict
D.dcm(A)
# check that _dcm_cache's have the D <-> A relationship defined (but not _dcm_dict)
A.dcm(D)  # check to make sure result is pulled from the cache, not recomputed
A.orient(B)  # change orientation of A <-> B
# check that _dcm_cache has been wiped and now equals _dcm_dict
# etc

I personally still don't know why there is a dcm_cache and a dcm_dict. If you understand that, could you make some comments here explaining it.

Yes, I would try

@moorepants I think _dcm_cache and _dcm_dict both already have some comments explaining them.

What does that mean and why are their two things?

For example, this sentence "The _dcm_cache dictionary will work as the dcm cache. " means nothing. It's like saying "The cow is the cow".

If you change something in _dcm_dict then I guess that means you have to clear anything in the cache that could be affected, or maybe it just gets wiped completely if _dcm_dict changes. The explanation I'm wondering about would answer these questions.

In ._dcm() if we don't have relation already defined with parent

# Example
B.orient(A)
C.orient(A)
D.orient(C)
E.orient(C)

then in both _dcm_dict and _dcm_cache new relation is added.

But if a user do something like

B.orient(A,..)
A.orient(B,..)

then both _dcm_dict and _dcm_cache of A is cleared and new relation with B is added on the other hand _dcm_dict and _dcm_cache of frame B is updated with new relation with A while other relations of frame B stay intact.

I still don't know why more than one data structure is needed.

Let's say we have a dictionary that stores the transformations of any adjacent (directly connected) frames. It works like this:

>>> B.orient(A)
>>> B.adjacent_relations
{A: A_dcm_B}
>>> A.adjacent_relations
{B: B_dcm_A}
>>> C.adjacent_relations
{}
>>> C.orient(A)
>>> A.adjacent_relations
{B: B_dcm_A, C: C_dcm_A}
>>> B.adjacent_relations
{A: A_dcm_B}
>>> C.adjacent_relations
{A: A_dcm_C}

Now try to change existing relationships:

>>> C.orient(B)
Error! you can't do this because it closes the loop and it's ambiguous what relation to delete to open the loop
>>> A.orient(B)  # replaces the A<->B relationship, no problem
>>> A.adjacent_relations
{B: B_dcm_A_new, C: C_dcm_A}
>>> B.adjacent_relations
{A: A_dcm_B_new}
>>> C.adjacent_relations
{A: A_dcm_C}

The A.orient(B) simply needs to update the adjacent_relations dict with the new relationship, nothing else should change.

So why is a _dcm_cache needed, what is that doing in this scenario?

You wrote:

print(A._dcm_cache) # Calculated dcm wrt C is added to _dcm_cache, no effect on _dcm_dict.

Earlier I wrote:

_dcm_cache can have everything that is in _dcm_dict but can also store relationships between self and any other reference frame that has a relative relationship (even if through a chain of reference frames).

So, is that not the same? That the cache stores relationships that are not adjacent? If you then change an adjacent relationship, you better wipe the cache because it could affect that.

I still don't know why more than one data structure is needed.

@moorepants I too believe we can merge _dcm_cache and _dcm_dict. _dcm_cache is only being used in frame.dcm(). _dcm_cache functionality could be added to _dcm_dict.

The A.orient(B) simply needs to update the adjacent_relations dict with the new relationship, nothing else should change.
So why is a _dcm_cache needed, what is that doing in this scenario?

_dcm_cache's sole purpose is in frame.dcm() for which _dcm_cache needs to be treated as _dcm_dict but with extra feature of storing calculated dcm.

So, is that not the same? That the cache stores relationships that are not adjacent?

_dcm_cache stores adjacent relationship along with those which are not adjacent but only when user explicitly calls dcm(). The dcm calculated between 2 frames adjacent or not is stored in _dcm_cache along with all adjacent ones.

If you then change an adjacent relationship, you better wipe the cache because it could affect that.

Yes that's True and this the default behavior in this PR.

Would "adjacent" be better than "immediate"? I don't really know what immediate means in this context.

By immediate I mean, chain parents-child relations won't be stored.

A.orient(B)
C.orient(A)

Then C._dcm_dict won't have dcm wrt B

@moorepants I think "adjacent" would be better.

I still don't understand this last sentence. What is different in _dcm_cache than _dcm_dict?

@moorepants _dcm_cache stores everything inside _dcm_dict but also stores calculated dcm value of that frame.
If we do something like

A.oreint(B)
B.orient(C)

then for frame C _dcm_dict and _dcm_cache would have nothing related to frame A, but we do have a connecting path between A and C.

So now if we do

C.dcm(A) or A.dcm(A)

then this calculated dcm would be stored in _dcm_cache and won't effect _dcm_dict.

Why would A have a relationship to A?

I don't know why is it behaving like this, I tried to dig but till now I'm not sure why is it happening but I do know what is happening .

I do know what is happening .

@moorepants If we pass a frame as parent frame in any orient() then that frame's _dcm_cache stores {self: self.dcm(self)} i.e dcm value wrt itself.

Does it store the identity matrix?

Yes, it does:

In [14]: A._dcm_cache
Out[14]: 
{A: Matrix([
 [1, 0, 0],
 [0, 1, 0],
 [0, 0, 1]]),
 B: Matrix([
 [1,      0,       0],
 [0, cos(4), -sin(4)],
 [0, sin(4),  cos(4)]]),
 D: Matrix([
 [                       cos(4)**2,                   -sin(4)*cos(4),         sin(4)],
 [sin(4)**2*cos(4) + sin(4)*cos(4),            cos(4)**2 - sin(4)**3, -sin(4)*cos(4)],
 [   -sin(4)*cos(4)**2 + sin(4)**2, sin(4)**2*cos(4) + sin(4)*cos(4),      cos(4)**2]])}