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

fix bug in select for single string antenna_name, flatten higher-dimensional arrays in select #390

Merged
merged 3 commits into from Jul 6, 2018

Conversation

bhazelton
Copy link
Member

Fixes a bug where if a single antenna_name string was passed it treated the characters in the string as separate antenna names rather than using the whole string as the antenna name.

Also add asserts to prevent problems with nested lists selects.

@adampbeardsley
Copy link
Member

I think asserting that ndim=1 is fine. Are there any use cases where people would supply larger dim arrays and we should just flatten?

adampbeardsley
adampbeardsley previously approved these changes Jul 5, 2018
@adampbeardsley
Copy link
Member

@bhazelton I went ahead and approved this, but I'll leave it to you to merge pending what you think about my above comment.

@bhazelton
Copy link
Member Author

Hmm. I guess we could just flatten them. The problem I was trying to fix was that the error you got with higher dimensionality was completely confusing, but flattening would fix that.

I guess the question is: is there any situation in which flattening is the wrong behavior? I'm struggling to think of one.

@adampbeardsley
Copy link
Member

Yeah, I'm struggling to think of either a case where flattening is wrong, or an actual use case where higher dim arrays would be input. Either way, we should probably make the assertion error more clear. I think as is it just gives:
AssertionError:

@bhazelton
Copy link
Member Author

Ok, maybe I'll just change it to flatten the arrays. Then I think I don't need the assert.

@bhazelton
Copy link
Member Author

Does the flatten ordering matter?

@bhazelton bhazelton changed the title fix bug in select for single string antenna_name, add asserts fix bug in select for single string antenna_name, flatten higher-dimensional arrays in select Jul 6, 2018
@bhazelton
Copy link
Member Author

@adampbeardsley I changed all the asserts to flattens. Can you take another look when you get a chance?

Copy link
Member

@adampbeardsley adampbeardsley left a comment

Choose a reason for hiding this comment

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

Sorry, I thought I approved the update before I left work yesterday. All looks good!

@adampbeardsley adampbeardsley merged commit 329e9f4 into master Jul 6, 2018
@adampbeardsley adampbeardsley deleted the select_bug_fix branch July 6, 2018 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants