Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Joss review response #4

Merged
merged 72 commits into from
Jan 30, 2024
Merged

Joss review response #4

merged 72 commits into from
Jan 30, 2024

Conversation

nbwuzhe
Copy link
Member

@nbwuzhe nbwuzhe commented Sep 27, 2023 •
edited
Loading

This PR is for responding to the review comments of the manuscript to JOSS.

@nbwuzhe
Copy link
Member Author

nbwuzhe commented Sep 27, 2023

Hi @felixhorger, thanks for reviewing our manuscript to JOSS. I have changed README.md to address both of your comments here. Would also appreciate input from the other authors.

@nbwuzhe nbwuzhe mentioned this pull request Sep 27, 2023
Closed
@alexjaffray
Copy link
Collaborator

daa1608 addresses #11

@alexjaffray
Copy link
Collaborator

06c5e81, aff10d5, e81f4e4 fix #6, #5, and #8 respectively

@alexjaffray
Copy link
Collaborator

Renamed variables, holding off on renaming the functions just yet @nbwuzhe @mrikasper

@alexjaffray
Copy link
Collaborator

alexjaffray commented Oct 30, 2023 •
edited
Loading

List of functions needing automated tests:

  • getSliceOrder
  • syncTrajAndData!
  • do_k0_correction!
  • adjustHeader!
  • checkAcquisitionNodes!
  • validateSiemensMRD!
  • validateAcqData!
  • preprocessCartesianData
  • removeOversampling!
  • mergeRawInterleaves
  • applyGIRF!
  • applyK0!
  • saveMap
  • loadMap
  • shift_kspace!

@alexjaffray
Copy link
Collaborator

10761b8 addresses #10

@alexjaffray
Copy link
Collaborator

alexjaffray commented Nov 14, 2023 •
edited
Loading

List of functions needing automated tests:

  • getSliceOrder
  • syncTrajAndData!
  • do_k0_correction!
  • adjustHeader!
  • checkAcquisitionNodes!
  • validateSiemensMRD!
  • validateAcqData!
  • preprocessCartesianData
  • removeOversampling!
  • mergeRawInterleaves
  • applyGIRF!
  • applyK0!
  • saveMap
  • loadMap
  • shift_kspace!

Need to update for new function names

@alexjaffray
Copy link
Collaborator

alexjaffray commented Nov 29, 2023 •
edited
Loading

Updated function list to track test writing progress:

Utilities:

  • calculate_b0_maps - Alex
  • get_slice_order - Tim
  • sync_traj_and_data! - Alex
  • do_k0_correction! - Alex
  • adjust_header! - Tim
  • check_acquisition_nodes! - Alex
  • validate_siemens_mrd! - Alex
  • validate_acq_data! - Alex
  • preprocess_cartesian_data -Alex
  • remove_oversampling! - Lars
  • save_map - Tim
  • load_map - Tim
  • shift_kspace! - Alex

Integration Tests:

  • merge_raw_interleaves - Tim and Alex and Lars
  • apply_girf! - Alex
  • apply_k0! - Alex

@alexjaffray alexjaffray mentioned this pull request Jan 16, 2024
Closed
@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2024 •
edited by codecov bot
Loading

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@alexjaffray alexjaffray mentioned this pull request Jan 30, 2024
Closed
3 tasks
alexjaffray
alexjaffray approved these changes Jan 30, 2024
View reviewed changes
Copy link
Collaborator

@alexjaffray alexjaffray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Significant progress towards revision response, approving to finalize doc testing and release soon

@alexjaffray alexjaffray merged commit 53c54d3 into main Jan 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexjaffray alexjaffray alexjaffray approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nbwuzhe @alexjaffray @codecov-commenter