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

#73 Add Kicktime to kick_$.tfs #77

Merged

Conversation

JoschD
Copy link
Member

@JoschD JoschD commented Jul 31, 2019

  • Refactored kick.py to use only constants as Column names
  • harpy/handler.py: added TIME to headers.
  • accelerator.py: refactored strings of element types to AccElementTypes class

  * Refactored kick.py to use only constants as Column names
  * harpy/handler.py: added TIME to headers.
  * accelerator.py: refactored strings of element types to AccElementTypes class
@JoschD JoschD requested review from awegsche and mihofer July 31, 2019 16:23
@mihofer
Copy link
Contributor

mihofer commented Aug 1, 2019

omc3/omc3/harpy/handler.py

Lines 145 to 158 in 4ad993a

def _compute_headers(panda, date):
headers = OrderedDict()
for plane in ALL_PLANES:
for prefix in ("", "NAT"):
try:
bpm_tunes = panda.loc[:, f"{prefix}TUNE{plane}"]
except KeyError:
pass
else:
headers[f"{prefix}Q{PLANE_TO_NUM[plane]}"] = np.mean(bpm_tunes)
headers[f"{prefix}Q{PLANE_TO_NUM[plane]}RMS"] = np.std(bpm_tunes)
if date is not None:
headers["TIME"] = date.strftime(formats.TIME)
return headers

why not use instead of just date as argument a dict containing additional headers, would allow to easily implement more headers at a later point if needed

@mihofer
Copy link
Contributor

mihofer commented Aug 1, 2019

def _get_header(header_dict, beta_d):
header = header_dict.copy()
header[RESCALE_FACTOR] = beta_d
return header

maybe change to RESCALE_FACTOR{plane}, kick files could then be merged without name clash

@mihofer
Copy link
Contributor

mihofer commented Aug 1, 2019

should optic_measurements/constants.py be put in definitions as other scripts (kmod, amp. det script, ..) should also rely on the conventions established there?

@JoschD
Copy link
Member Author

JoschD commented Aug 1, 2019

omc3/omc3/harpy/handler.py

Lines 145 to 158 in 4ad993a

def _compute_headers(panda, date):
headers = OrderedDict()
for plane in ALL_PLANES:
for prefix in ("", "NAT"):
try:
bpm_tunes = panda.loc[:, f"{prefix}TUNE{plane}"]
except KeyError:
pass
else:
headers[f"{prefix}Q{PLANE_TO_NUM[plane]}"] = np.mean(bpm_tunes)
headers[f"{prefix}Q{PLANE_TO_NUM[plane]}RMS"] = np.std(bpm_tunes)
if date is not None:
headers["TIME"] = date.strftime(formats.TIME)
return headers

why not use instead of just date as argument a dict containing additional headers, would allow to easily implement more headers at a later point if needed

A dict would make sense if the headers are added automatically. But as there is some formatting going on, one has to handle each entry individually anyway (most likely). Changing date to addition_headers later if needed should be no problem though.

@JoschD
Copy link
Member Author

JoschD commented Aug 1, 2019

should optic_measurements/constants.py be put in definitions as other scripts (kmod, amp. det script, ..) should also rely on the conventions established there?

Yes... but we need come to some convetion later. New issue, I guess

@JoschD
Copy link
Member Author

JoschD commented Aug 1, 2019

def _get_header(header_dict, beta_d):
header = header_dict.copy()
header[RESCALE_FACTOR] = beta_d
return header

maybe change to RESCALE_FACTOR{plane}, kick files could then be merged without name clash

Maybe. Also new issue to talk about.

@JoschD JoschD merged commit a99c79a into enhancement/13/optics_measurements Aug 1, 2019
@JoschD JoschD deleted the bug/73/kicktimes_in_kick_plane branch August 1, 2019 09:36
lmalina added a commit that referenced this pull request Aug 6, 2019
Closes #13, follow-ups: #30, #87, #88.

* #73 Add Kicktime to kick_$.tfs (#77)
* harpy/handler.py: added TIME to headers of lin-files
* Refactored kick.py to use only constants as Column names, added time from lin-files
* accelerator.py: refactored strings of element types to AccElementTypes class

* #64 Cleaned .gitignore

* #78 Added plane to RescaleFactor in kick.py

* #83 renamed --accelerator to --tbt_datatype

* #80: Squashed commit from bug/80/free_kick_compensation

* #69 Fixing bug in outliers

* #84 Added debug message for no combinations in N-BPM method
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