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.

Sign up for GitHub

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

Jump to bottom

[SYSTEMDS-3461] FrameBlocks Arrays Separation #1718

Closed
wants to merge 3 commits into from
Closed

[SYSTEMDS-3461] FrameBlocks Arrays Separation #1718

wants to merge 3 commits into from

Conversation

Baunsgaard
Copy link
Contributor

This commit is the first of updates to frame blocks,
It starts to move the logic of the encoded columns out of the FrameBlock.

@Baunsgaard
Copy link
Contributor Author

image

Current code coverage when running frame component and function tests

@Baunsgaard
Copy link
Contributor Author

image

Specific for frame Block.

@Shafaq-Siddiqi do you know where the tests for mapping is ? are they not located in frame component or functions tests?

@Shafaq-Siddiqi
Copy link
Contributor

image

Specific for frame Block.

@Shafaq-Siddiqi do you know where the tests for mapping is ? are they not located in frame component or functions tests?

they are inside functions/binary/frame

@Baunsgaard
Copy link
Contributor Author

they are inside functions/binary/frame

I will move them to functions/frame
unless you think that is bad?

@Shafaq-Siddiqi
Copy link
Contributor

they are inside functions/binary/frame

I will move them to functions/frame unless you think that is bad?

initially, I wanted to have them in separate packages with respect to the operation types, i.e., binary and ternary, but moving them all inside the functions/frame makes more sense.

@Baunsgaard
Copy link
Contributor Author

image

With map function tests.

@Baunsgaard Baunsgaard force-pushed the FrameColumns branch 2 times, most recently from 35b5ed0 to 0908d1d Compare November 11, 2022 10:37
@Baunsgaard
[SYSTEMDS-3461] FrameBlocks Arrays Separation
ff2b4fd 
This commit removes the tight binding of the underlying
arrays in frame block to its arrays for columns value types.
There is one binding missing that is to pipe frames to python.
This will be refactored in a following commit.

This further fixes the indentation of one of the frame test files
(tabs vs spaces) and moves the functional frame tests that was
placed in component tests to frames.
@Baunsgaard
[SYSTEMDS-3462] FrameBlock Iterators Factory Pattern
c899d7a 
This commit moves the iterators out of the frame, and to a factory pattern.
This is to in the future allow the customized column allocations to
iterate nicely, and therefore we need specialized code to return different
instances of iterators returned, and therefore to no bloat the code
internally in the FrameBlock we move this logic out.
@Baunsgaard
[MINOR] Move map function test
3c1bd6a 
This commit moves the map function tests for frames to functions/frame
instead of binary/frame, to colocate the frame testing.
@Baunsgaard
Copy link
Contributor Author

Hi @Shafaq-Siddiqi ,

I could use a review if you have time.

@Shafaq-Siddiqi
Copy link
Contributor

Hi @Shafaq-Siddiqi ,

I could use a review if you have time.

LGTM,
great work! I really appreciate the efforts you have put in.

@Baunsgaard Baunsgaard deleted the FrameColumns branch November 24, 2022 22:44
fathollahzadeh pushed a commit to fathollahzadeh/systemds that referenced this pull request Dec 7, 2022
@Baunsgaard @fathollahzadeh
[MINOR] Move map function test
2c13c14 
This commit moves the map function tests for frames to functions/frame
instead of binary/frame, to colocate the frame testing.

Closes apache#1718
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shafaq-Siddiqi Shafaq-Siddiqi Awaiting requested review from Shafaq-Siddiqi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@Baunsgaard @Shafaq-Siddiqi