refactor: new parametrized unpack decorator requires function signature with kwargs

[ConnorStoneAstro]

The unpack decorator now requires function signatures to be of the form:

func(self, arg1, arg2, ..., *args, params = None, param1 = None, param2 = None, ..., kwarg1 = val1, kwarg2 = val2, **kwargs)

This allows for some notable benefits:

• @unapck is all that's needed as the decorator, no longer @unpack(<number>)
• Can now identify which parameter is missing and provide helpful error message of the form: KeyError: "Missing parameter 'x0' in method 'jacobian_lens_equation' of module 'sie'"
• kwargs that aren't parameters can now get passed reliably without crashing
• unpack decorator is much simpler
• It is aware of the dynamic parameters needed and so behaves nicer when mixed static/dynamic are used

The only case we lose is parameters passed as args like this:

lens = SIE()
lens.raytrace(x,y,z_s,z_l,x0,y0,q,phi,b)

It would instead need to be:

lens = SIE()
lens.raytrace(x,y,z_s,z_l=z_l,x0=x0,y0=y0,q=q,phi=phi,b=b)

and it would work.

Cases where everything is packed would still work, even passed as an arg, so long as it's the last arg:

lens = SIE()
x = [1,1,1,1,1,1]
lens.raytrace(x,y,z_s, lens.pack(x))

That is just like before.

[codecov]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (bec792a) 89.34% compared to head (ddb955a) 89.37%.

Files	Patch %	Lines
src/caustics/lenses/base.py	93.75%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #136      +/-   ##
==========================================
+ Coverage   89.34%   89.37%   +0.03%     
==========================================
  Files          36       36              
  Lines        1942     1939       -3     
==========================================
- Hits         1735     1733       -2     
+ Misses        207      206       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AlexandreAdam]

Comments are about passing kwargs forward in method so that called method also have access to them

[AlexandreAdam]

I think this is a very good PR. As discussed, all that is needed now is to implement the change in every method, make sure kwargs are passed forward.

For documentation purposes, it would be nice if you could highlight some examples where this change impacts the API. In particular, how a user who wants to call directly a method without using the packed argument would interact with this feature and how it plays with dynamic/static parameters. Would make a neat jupyter notebook.

[AlexandreAdam]

Need to expand the scope of the tests

[AlexandreAdam]

My comments are centered around the usage of the pack method, which should be considered more as a private method than a public method in caustics.

[AlexandreAdam]

This is great!