parsotat / BatAnalysis

A python HEASOFT wrapper for processing Swift-BAT data.
MIT License
30 stars 11 forks source link

Issue with different types of timedelta in group_inventory #20

Closed Parrazyte closed 5 months ago

Parrazyte commented 6 months ago

The group_inventory function has an explicit comparison with a 1M timedelta64 (line 369)

if binning_timedelta==np.timedelta64(1,'M'):

Which crashes if the binning_timedelta argument is a timedelta from another unit (e.g., day).

Proposed fix:

if np.dtype(binning_timedelta)==np.dtype(timedelta64(1,'M')) and binning_timedelta==np.timedelta64(1,'M'):

prevents the issue

(note: I also had an issue with explicit np.int calls in group_outventory in mosaic.py, it seems it's been fixed in the github version but I assume that the pip version is not up to date?)

parsotat commented 6 months ago

HI @Parrazyte, what version on python and numpy do you have? I tried the first line and it executed successfully with a warning, so I wanted to make sure that is really the issue here.

My python is 3.11.3 and numpy version is 1.24.3

Parrazyte commented 6 months ago

I'm on python 3.10.12 and numpy 1.26.4

Here's what I get:

np.timedelta64(1,'D')==np.timedelta64(1,'M')
Traceback (most recent call last):
  File "/home/parrazyte/Documents/Work/PhD/Scripts/winds/venv/lib/python3.10/site-packages/IPython/core/interactiveshell.py", line 3577, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-3-e789e35938dd>", line 1, in <module>
    np.timedelta64(1,'D')==np.timedelta64(1,'M')
TypeError: Cannot get a common metadata divisor for Numpy datetime metadata [D] and [M] because they have incompatible nonlinear base time units.
parsotat commented 6 months ago

While, I make the necessary changes, try to downgrade your numpy to 1.24.3 and this will allow things to run as I had intended them to run. pip and condo allow for a specified version of a package to be installed.

parsotat commented 5 months ago

This was addressed via PR #21