Skip to content

Conversation

kakoon
Copy link

@kakoon kakoon commented May 23, 2025

No description provided.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@kakoon kakoon requested a review from b1quint May 23, 2025 16:54
@@ -0,0 +1,373 @@
{
Copy link
Member

@b1quint b1quint May 23, 2025

Choose a reason for hiding this comment

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

Can you remove this cell? It does not add anything to the execution.


Reply via ReviewNB

@@ -0,0 +1,373 @@
{
Copy link
Member

@b1quint b1quint May 23, 2025

Choose a reason for hiding this comment

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

May I ask you to reformat this part of the code? Maybe add spaces between logical blocks of code. For example, instead of:

lews_selected = []
hp_max_hist = np.array([])
hp_spread_max_hist = np.array([])
min_ele_range = -1  # minimum elevation change in slew, degrees
min_azi_range = -1  # minimum azimuth change in slew, degrees
hp_threshold = OPERATIONAL_LIMIT
velocity_threshold = 0.5 # deg/s
df_state = getEfdData(
    client,
    "lsst.sal.MTM1M3.logevent_detailedState",
    begin=Time(slews[0].begin, format="isot", scale="utc"),
    end=Time(slews[-1].end, format="isot", scale="utc"),
) 

Add a space to separate the variable definition from the query that is a multi-line code. Something like this:

lews_selected = []
hp_max_hist = np.array([])
hp_spread_max_hist = np.array([])
min_ele_range = -1  # minimum elevation change in slew, degrees
min_azi_range = -1  # minimum azimuth change in slew, degrees
hp_threshold = OPERATIONAL_LIMIT
velocity_threshold = 0.5 # deg/s

df_state = getEfdData(
    client,
    "lsst.sal.MTM1M3.logevent_detailedState",
    begin=Time(slews[0].begin, format="isot", scale="utc"),
    end=Time(slews[-1].end, format="isot", scale="utc"),
) 

But apply this to the whole cell. Otherwise, it is hard to read.


Reply via ReviewNB

Copy link
Author

Choose a reason for hiding this comment

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

done, pushed again!

@b1quint
Copy link
Member

b1quint commented May 23, 2025

The notebook execution is failing. You can find it by clicking on the "Notebook Execution" above and then clicking on your notebook. Here is a shortcut with the traceback:

https://usdf-rsp.slac.stanford.edu/times-square/github-pr/lsst-sitcom/reports-performance-summary/061c378bccd80255cd7e1374e969bb3c2f683cf3/sst/mtm1m3/M1M3_max_forces_histogram

@b1quint b1quint self-requested a review May 23, 2025 19:22
Copy link
Member

@b1quint b1quint left a comment

Choose a reason for hiding this comment

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

This is working good! Thank you very much!

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