fromExcel Enhancements and add dataset.toDict#48
Conversation
|
Added toDict. This would be useful for multiple applications, such as separating a dataset by columns and extracting values into an array. fromExcel( toDict( |
|
Very cool! I'm definitely on board with enhancements/bug fixes. I think an approach that might more sense than a strict "string these columns" override would be the ability to specify any type overrides? Maybe in a dictionary form of If you're not using IntelliJ, I'd recommend it. If you install the Ktlint plugin, it'll help clean up the formatting issues that will currently fail this PR if I run the github actions. In addition to that, we'll want to clean up the unnecessary committed If you're not comfortable making any of those changes, I'm happy to steer this PR a little - I really appreciate anyone making contributions, and don't want to bog you down with process :) If you do want to try, I'm also happy to help you out with the process. |
|
Hey, thanks so much for the response. I currently use VSCode, so there is no formatting or code feedback. It has been a lot of googling, compiling, and troubleshooting. I'll plan on setting up a better environment to code tomorrow. Would you happen to have any recommendations for clearing the bin files besides deleting the folder? I am currently running the following commands when I want to create a new
Feel more than free to help with this. I would appreciate it. Today is day two of Kotlin for me. You won't be stepping on any toes here 😂 I like your suggestion of I am unsure what to optimize in Once again, thanks for the response. I love this project and think it is a great idea; hopefully, I can help. |
Those should be the correct commands, but gradle uses the I went away and cleaned up the PR - I dropped the bin/ files that were added and tweaked the implementations of toDict and fromExcel. If you're able to run these and test them (or provide me any files you're using to test so I can add more unit tests) that would be much appreciated. If you'd like me to go into further detail on the changes I've made, I'm also happy to do that :) |
| dataset.getColumnName(col) to PyList( | ||
| buildList { | ||
| for (row in dataset.rowIndices) { | ||
| val value = dataset[row, col] | ||
| if (value != null || !filterNull) { | ||
| add(value) | ||
| } | ||
| } | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Technically this allocates one unnecessary Pair instance for each iteration, but since the actual heavy object here will be the list of column data, I think this is fine. getColumnAsList would be helpful here, but then you'd have to filter the list as a second operation, meaning two scans of the same column instead of just one.
|
Okay, so I ran into an issue I can't solve being |
|
Wouldn't we want the parameter to be a |
|
Test case is passing now. It was an issue with |
|
I think typeOverrides will have to accept a dictionary where the keys are just column indexes (within the clump of data to be parsed). Using string names is too fragile. |
This pull request would add two new features to the
fromExcelfunction.Use case example
Because the Desc is blank for 4A, this would throw a null pointer exception.
Use case example
Because 44 is the first value, the table column would be assigned
Int::class.javaObjectType; this would cause 4A to throw an error. I added a new parameter named stringColumns. stringColumns takes in a list of column positions to String.I am new to GitHub (I don't know how to merge this properly) and Kotlin (code may not be as precise as needed). The files changed where