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.

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

Updated docstrings for k-gain and non-linearity calibration #216

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

hsergi
Copy link
Contributor

@hsergi hsergi commented Oct 12, 2024

Describe your changes

As part of the effort to review the doc in the calibration functions before moving to e2e simulations, I have updated docstrings for calibrate_kgain.py and calibrate_nonlin.py

Type of change

I have updated the main docstrings of calibrate_kgain.py and calibrate_nonlin.py describing what the data structure is in a common fashion for both calibration functions. The same information can be found on Confluence (link). It has been run thru Guillermo Gonzalez.

I also fixed a condition on the number of frames used to calibrate non-linearity in calibrate_nonlin.py

I have checked that e2e tests pass the same way as before the docstring changes.

Checklist before requesting a review

  • [x ] I have verified that all unit tests pass in a clean environment and added new unit tests, as needed
  • I have verified that all docstrings are properly formatted and added new documentation, as needed

@semaphoreP
Copy link
Contributor

Should we wait until we have a discussion on new header formats before finalizing the updated docstring @hsergi?

@hsergi
Copy link
Contributor Author

hsergi commented Oct 19, 2024

Sounds good. Marie asked me to work&submit this review that is compatible with v1.0. It clarifies a bit the structure of the cal data. We can wait, especially once we know how much work is required to update the use of the keywords.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants