search

TopEFT / topeft

15 stars 24 forks source link

Need to replace np.bool by bool to avoid Numpy Error #360

Closed klannon closed 1 year ago

klannon commented 1 year ago

We need to replace any use of np.bool by just bool or else our code doesn't run under the most recent version of numpy. These don't happen so often. Looks like it happens only in two files.

https://github.com/TopEFT/topcoffea/search?q=np.bool

@hannahbnelson @ywan2 Can one of you make this fix?

ywan2 commented 1 year ago

Sure. I can fix it. A lot of recent updates are getting done on technical_improvements branch. I am wondering if we should apply the above changes on master branch or technical_improvements branch?

klannon commented 1 year ago

That's a great question! Maybe @kmohrman @bryates or @sscruz has an opinion. We might need to apply in both places to avoid the CI eventually breaking when the numpy version increments there.

kmohrman commented 1 year ago

I agree that it would make sense to make a PR to both of the branches (as it is not clear when the technical improvements branch will be merged).

ywan2 commented 1 year ago

Got it. Will do. Thank you!

kmohrman commented 1 year ago

By the way, just as a heads up: you will also need the latest version of coffea 0.7.21 (since older versions of coffea also have instances of np.bool that lead to this error, e.g. I ran into it again in one of my older environments that had coffea 0.7.20).

ywan2 commented 1 year ago

I fixed all the np.bool on both master and technical_improvements branch. It should be working now.

klannon commented 1 year ago

Fixed by 91d0f690f631932a0718435548d9080366a3c146 and 08b21e1. (@ywan2 : Always good to note which commit or PR fixes the issue, and then close it when it's fixed.)

klannon commented 1 year ago

But thanks for fixing it!