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

add conditions in the function of perpendicular_momentum #8

Merged
merged 5 commits into from
Oct 8, 2024

Conversation

Jiemin-Li
Copy link
Contributor

Add some conditions at the beginning of the perpendicular_momentum such that (1) the input arguments must have same lengths; (2) or there is only one input argument whose length is larger than 1. @awalter-bnl Please have a look.

Copy link
Contributor

@awalter-bnl awalter-bnl left a comment

Choose a reason for hiding this comment

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

Perhaps this could be written a little more succinctly written as below. I think this will run faster than the multiple if statements that you are suggesting. But I let you decide, I have made a few other comments if you decide not to use what is below.

check_data = [len(value)
      if (isinstance(value,(np.ndarray)) and len(value.shape)==1)
      else False
      for value in [inner_value for inner_value in 
                    [parallel_momentum, photon_energy, 
                     binding_energy]
                    if not isinstance(inner_value, (float, int))]]

if not np.all(check_data_len):
    raise ValueError (f'Some string about not being an int, float or 1D numpy array')
elif check_data_len[:-1] != check_data_len[1:]:
    raise ValueError (f'Some string about all numpy arrays not being the same length')

src/test_data/arpes.py Outdated Show resolved Hide resolved
else:
if len(i)>1:
check_data_len.append(len(i))
elif isinstance(i,(list)):
Copy link
Contributor

Choose a reason for hiding this comment

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

This elif is not required, as inputting these as lists will not work. The reason for this is the code below uses numpy arithmetic functions which are only guaranteed to work for numpy arrays, floats or ints. If you would like lists to work add some code before this that checks for lists in these inputs and convert them to numpy arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replace these lines with the new codes.

@awalter-bnl awalter-bnl merged commit 4bd3828 into NSLS-II-ARI:main Oct 8, 2024
3 of 4 checks passed
@awalter-bnl
Copy link
Contributor

Looks good @Jiemin-Li , thanks for the PR

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