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 inscopix dtype #326

Merged
merged 6 commits into from
May 18, 2024
Merged

fix inscopix dtype #326

merged 6 commits into from
May 18, 2024

Conversation

bendichter
Copy link
Contributor

Apparently np.uint16 is different from np.dtype(np.uint16)

@CodyCBakerPhD
Copy link
Member

Specifically seems that if you ever see numpy.uint16 it's from taking the type instead of the .dtype of an array element or numpy scalar;

array = np.array([1,2,3], dtype="uint16")
type(array[0])
> np.uint16

array[0].dtype
> dtype('uint16')

@CodyCBakerPhD
Copy link
Member

Is there a test to add or fix (doesn't look like changing this line in source altered the outcome of a test?) for this

@bendichter
Copy link
Contributor Author

Unfortunately,

np.uint16 == np.dtype(np.uint16)
True

however

np.uint16.itemsize()
TypeError: 'getset_descriptor' object is not callable

while

np.dtype(np.uint16).itemsize
2

I suppose I could test asking for the itemsize

@CodyCBakerPhD
Copy link
Member

Unfortunately,

np.uint16 == np.dtype(np.uint16)

I believe this might be intended since equality is somewhat looser than saying each side of the expression should be of the same object type (with same methods/attributes, etc)

I think the correct assertion would instead be

assert extractor.get_dtype() is np.dtype("uint16")

@bendichter
Copy link
Contributor Author

@CodyCBakerPhD good point. I changed it to is and it works. You are right, is should be more restrictive

@CodyCBakerPhD CodyCBakerPhD enabled auto-merge (squash) May 18, 2024 17:08
@CodyCBakerPhD CodyCBakerPhD merged commit b5952ca into main May 18, 2024
25 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the fix_inscopix_dtype branch May 18, 2024 17:12
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