RNourshargh / CavitySim

This Matlab code simulates different optical cavities using ABCD matricies from the constituent optical components.
MIT License
0 stars 0 forks source link

Fix master #8

Closed znicholls closed 5 years ago

znicholls commented 5 years ago

I finally got around to this.

expected_radii_lists and result_radiilist are lists of lists. However, their elements are all different lengths

> [len(i) for i in expected_radii_lists]
[4, 8, 6]
> [len(i) for i in result_radiilist]
[4, 8, 6]

This means that numpy won't cast them to arrays, rather a yuck array of lists.

> np.array(expected_radii_lists)
array([list([0.00016931952799938106, 0.00016931952799938106, 0.00016931952799938106, 0.00016931952799938106]),
       list([0.0002073732235436865, 0.0002073732235436865, 0.000414746447087373, 0.000414746447087373, 0.0002073732235436865, 0.0002073732235436865, 0.000414746447087373, 0.000414746447087373]),
       list([0.0006050383556457971, 0.000605038355645797, 0.0005819072571444502, 0.0005819072571444502, 0.0005819072571444503, 0.0005819072571444503])],
      dtype=object)

Hence np.testing.assert_allclose doesn't know what to do as it doesn't know how to compare lists, only arrays. The solution is to just loop over the elements in expected_radii_lists before passing them to np.testing.assert_allclose.

To work this out, I took these steps:

  1. look at the output of np.array(expected_radii_lists)
  2. realise it wasn't giving back a proper array
  3. google 'numpy array from list of lists'
  4. look at stack overflow
  5. top answer made the problem clear
RNourshargh commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@b224d11). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master       #8   +/-   ##
=========================================
  Coverage          ?   96.35%           
=========================================
  Files             ?        2           
  Lines             ?      137           
  Branches          ?       11           
=========================================
  Hits              ?      132           
  Misses            ?        5           
  Partials          ?        0

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 b224d11...5a169dd. Read the comment docs.

RNourshargh commented 5 years ago

Thank you so much for this! I wanted to make sure I properly understood what was going on and had read the stack overflow page before merging which I have now done. This might just be an impression but it looks like a very unpythonic way of doing things. I won't change this now, but is there a more natural structure I can use for tests of this kind? Thanks again R

znicholls commented 5 years ago

I won't change this now, but is there a more natural structure I can use for tests of this kind?

pytest's parameterisation is best. However this function returns a list of arrays so I'm not sure you can test it any better unless you want to change the return value...