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 incorrect region number counting in regions_pyregi… #126

Merged
merged 1 commit into from
May 9, 2017

Conversation

leejjoon
Copy link
Contributor

@leejjoon leejjoon commented May 8, 2017

in dev/regions_pyregion_comparison.py, the number of regions are calculated by counting the occurence of "(". This overestimates for the following cases

panda(18:13:25.141,+31:21:31.32,51.8442,90.2623,1,0",4.995",1) # panda=(51.8442 90.2623 180.262 270.262)(0" 4.995" 9.99") text={Panda 2}

The patch only counts "(" not preceded by "=" or ")".

@keflavich
Copy link
Contributor

Thanks for noticing this. I was trying to find an example where overcounting would happen and I didn't see any in the test data we had, but this would also be a problem for any region with ( in the text={} box.

@leejjoon
Copy link
Contributor Author

leejjoon commented May 8, 2017

Yes, the proposed change can still count "(" in the text box. But as far as the coverage test is concerned, none of them has this.

@joleroi joleroi changed the title attempt to fix the incorrect region number counting in regions_pyregi… Fix incorrect region number counting in regions_pyregi… May 9, 2017
@joleroi joleroi merged commit 354959d into astropy:master May 9, 2017
@cdeil cdeil modified the milestones: 0.2, 0.3 Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants