Skip to content

fix: Support structured arrays in option type to_numpy#3060

Merged
jpivarski merged 4 commits into
mainfrom
to_numpy_option_record_array
Mar 25, 2024
Merged

fix: Support structured arrays in option type to_numpy#3060
jpivarski merged 4 commits into
mainfrom
to_numpy_option_record_array

Conversation

@zonca

@zonca zonca commented Mar 22, 2024

Copy link
Copy Markdown
Collaborator
  • [test] implement unit test for to_numpy on option-type of record type
  • [fix] handle structured arrays for option-type

See #3056

@zonca
zonca requested a review from jpivarski March 22, 2024 06:50
@zonca zonca changed the title Support structured arrays in option type to_numpy [fix] Support structured arrays in option type to_numpy Mar 22, 2024
@zonca zonca self-assigned this Mar 22, 2024
@zonca
zonca force-pushed the to_numpy_option_record_array branch 3 times, most recently from bce6167 to f651e9a Compare March 22, 2024 07:05
@zonca zonca changed the title [fix] Support structured arrays in option type to_numpy fix: Support structured arrays in option type to_numpy Mar 22, 2024
@zonca
zonca force-pushed the to_numpy_option_record_array branch 2 times, most recently from e1ef8ac to 3754ede Compare March 22, 2024 07:13
@ManasviGoyal

Copy link
Copy Markdown
Collaborator

@zonca Instead of adding the test to an existing test file, you should add a new test according to the contributing guidelines.

tests/test_XXXX-yyyy.py
where XXXX is either the number of the issue your pull request fixes or the number of the pull request and yyyy is descriptive text

@zonca
zonca force-pushed the to_numpy_option_record_array branch 2 times, most recently from 903284c to 66c491a Compare March 25, 2024 16:36
zonca added 3 commits March 25, 2024 16:38
Factored the functionality of creating missing data in separate
function,
call it for each field in a structured array.

Closes #3056
@zonca
zonca force-pushed the to_numpy_option_record_array branch from 66c491a to db39ce1 Compare March 25, 2024 16:38
@zonca

zonca commented Mar 25, 2024

Copy link
Copy Markdown
Collaborator Author

@zonca Instead of adding the test to an existing test file, you should add a new test according to the contributing guidelines.

tests/test_XXXX-yyyy.py
where XXXX is either the number of the issue your pull request fixes or the number of the pull request and yyyy is descriptive text

Thanks @ManasviGoyal, fixed that

@jpivarski jpivarski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That does it, thank you! Since this appears to be a complete solution, with test, and I don't think you're adding any more to it, I'm going to merge it now.

@jpivarski
jpivarski merged commit bc559e7 into main Mar 25, 2024
@jpivarski
jpivarski deleted the to_numpy_option_record_array branch March 25, 2024 19:24
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.

3 participants