sigpy icon indicating copy to clipboard operation
sigpy copied to clipboard

Add option for full-spoke radial trajectory

Open fyrdahl opened this issue 2 years ago • 3 comments

I noticed that the radial trajectory code defaulted to half-spoke (center-out) with no option to use full spokes, so I added that option. The default behavior doesn't change, but the new option is a closer match to what is described in the referenced papers. The change to beta in the full-spoke 3D-radial case is not strictly necessary, but the change reflects how it's described in the original paper.

I also changed the deprecated np.float to np.float32, as I understand you prefer explicit size instead of the built-in types.

fyrdahl avatar Aug 15 '22 10:08 fyrdahl

Any ideas why the checks failed? Not much info available from travis ci without a login?

curtcorum avatar Jan 11 '23 21:01 curtcorum

It's a bunch of failed flake8 tests, most of which seem unrelated to this PR. The failed test which I believe is related is an E501 line too long.

./sigpy/mri/samp.py:90:80: E501 line too long (83 > 79 characters)

Since most errors are unrelated to this PR, fixing the PR itself probably won't solve the failed checks. I think PEP8 should be abided by when possible but breaking up the argument list into multiple lines is a departure from the coding style in the rest of the code base. This will probably become a bigger problem when someone gets around to fixing the deprecated datatypes, which will add 2-3 chars to every line that specifies a dtype.

fyrdahl avatar Jan 11 '23 21:01 fyrdahl

Hi, sorry -- I updated the tooling. Can you please run bash run_tests.sh?

sidward avatar May 27 '23 21:05 sidward