Skip to content

test_numpy_dtypes: Add test for py::vectorize() - #2260

Merged
EricCousineau-TRI merged 6 commits into
pybind:masterfrom
EricCousineau-TRI:issue-2258-repro
Sep 17, 2020
Merged

test_numpy_dtypes: Add test for py::vectorize()#2260
EricCousineau-TRI merged 6 commits into
pybind:masterfrom
EricCousineau-TRI:issue-2258-repro

Conversation

@EricCousineau-TRI

@EricCousineau-TRIEricCousineau-TRI commented Jun 21, 2020•
edited
Loading

Copy link
Copy Markdown
Collaborator

Shows test for #2258

Feel free to close if this doesn't add value.

With the latest push of 337dc9c, I think this has value?

\cc @chaitan94

@EricCousineau-TRIEricCousineau-TRI mentioned this pull request Jun 21, 2020
Open
chaitan94
chaitan94 reviewed Jun 22, 2020
View reviewed changes
Comment threadtests/test_numpy_dtypes.py
assert m.f_simple(
m.SimpleStruct.fromtuple(s_recarray_scalar.item())) == 20

# Show that an array of dtype=object does *not* convert.

@chaitan94chaitan94Jun 22, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Exactly. This is what #2258 is about. I feel ideally m.f_simple_vectorized(np.array([s])) should work directly without any issues. If not, the docs should at least mention either the totuple/fromtuple or the creat_recarray-like workarounds.

@EricCousineau-TRIEricCousineau-TRISep 17, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Thanks for the comment! I think a docs change is currently out-of-scope for this PR (as it's a strict improvement).

Might you be up for trying out a doc PR? (if not, I can circle back to it, but it may take me a bit)

@EricCousineau-TRIEricCousineau-TRI mentioned this pull request Jul 1, 2020
Closed
@henryiiihenryiii reopened this Aug 18, 2020
henryiii
henryiii requested changes Aug 18, 2020
View reviewed changes
Comment threadtests/test_numpy_dtypes.cpp Outdated
@henryiii@chaitan94
Update tests/test_numpy_dtypes.cpp
416202d
Co-authored-by: Krishna Chaitanya <bkchaitan94@gmail.com>
@bstaletic

bstaletic commented Sep 16, 2020

Copy link
Copy Markdown
Collaborator

The new test is failing after rebase.

@henryiii

henryiii commented Sep 16, 2020

Copy link
Copy Markdown
Collaborator

I think that finally makes GCC 4.8 / CentOS 7 (manylinux1) happy.

@EricCousineau-TRI

EricCousineau-TRI commented Sep 17, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Awesome, sorry for the delay here, but merging now!

@EricCousineau-TRI

EricCousineau-TRI commented Sep 17, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Thank y'all!

@EricCousineau-TRI
EricCousineau-TRI merged commit 4e7c08d into pybind:masterSep 17, 2020
@rwgkrwgk mentioned this pull request Feb 10, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henryiiihenryiiihenryiii approved these changes
+2 more reviewers
@chaitan94chaitan94chaitan94 left review comments
@bstaleticbstaleticbstaletic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@EricCousineau-TRI@bstaletic@henryiii@chaitan94