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

Added ability to remove nil from DataFrame and Series #102

Merged
merged 3 commits into from Jul 11, 2019

Conversation

@AtharvaKhare
Copy link
Contributor

@AtharvaKhare AtharvaKhare commented Jun 19, 2019

DataSeries can remove nils using the method removeNils
DataFrame can remove nils from rows and columns using methods:

  • removeColumnsWithNilsAtRow:
  • removeColumnsWithNilsAtRowNamed:
  • removeRowsWithNilsAtColumn:
  • removeRowsWithNilsAtColumnNamed:
    Added relevant tests for these methods
@AtharvaKhare
Copy link
Contributor Author

@AtharvaKhare AtharvaKhare commented Jun 19, 2019

Any idea why code coverage decreased? The code in question is already covered in a test

Loading

@olekscode
Copy link
Member

@olekscode olekscode commented Jun 21, 2019

This seems like a bad implementation:

DataFrameInternal >> removeRowsWithNilsAtColumn: columnNumber
    "Removes all rows having a nil value at the column columnNumber"

     | newContents rowsToDrop k |
    "rowsToDrop has 1 at i if i-th row needs to be dropped, else 0"
    rowsToDrop := (self columnAt: columnNumber) collect: [ :ele |
        (ele isNil) ifTrue: [ 1 ] ifFalse: [ 0 ] ].
    newContents := Array2D
        rows: (self numberOfRows - (rowsToDrop select: [ :ele | ele = 1 ]) size)
        columns: (self numberOfColumns).

     1 to: self numberOfColumns do: [ :j |
        k := 0.
        1 to: self numberOfRows do: [ :i |
            (rowsToDrop at: i) = 1 
                ifTrue: [ k := k + 1 ]
                ifFalse: [
                newContents at: i - k at: j put:
                    (contents at: i at: j) ]]].

     contents := newContents.

First you collect 0 and 1 (instead of true and false), then you check if element is equal to 1 (instead of having the boolean element). And so on

Loading

@olekscode
Copy link
Member

@olekscode olekscode commented Jun 21, 2019

I suggest that we create a method for removing rows that satisfy a certain condition (and the same for columns).
Then you call it with the condition that element at a certain column is nil

Loading

DataSeries can remove nils using the method `removeNils`
DataFrame can remove nils from rows and columns using methods:
- `removeColumnsWithNilsAtRow:`
- `removeColumnsWithNilsAtRowNamed:`
- `removeRowsWithNilsAtColumn:`
- `removeRowsWithNilsAtColumnNamed:`
Added relevant tests for these methods

DataFrame can also delete rows and columns satisfing a certain
condition, such as isNil (above one), using these methods:
- `removeRowsOfColumnElementsSatisfing: onColumn:`
- `removeRowsOfColumnElementsSatisfing: onColumnNamed:`
- `removeColumnsOfRowElementsSatisfing: onRow:`
- `removeColumnsOfRowElementsSatisfing: onRowNamed:`
Similar method is available for DataFrameInternal
@AtharvaKhare
Copy link
Contributor Author

@AtharvaKhare AtharvaKhare commented Jun 22, 2019

@olekscode done! Any idea about coveralls? Tests are written :/

Loading

@olekscode olekscode merged commit c571730 into PolyMathOrg:master Jul 11, 2019
0 of 2 checks passed
Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants