search

vallettea / koala

Transpose your Excel calculations into python for better performances and scaling.
GNU General Public License v3.0
147 stars 59 forks source link

Spreadsheets can be loaded from file-like objects and random functions #212

Closed doconix closed 5 years ago

doconix commented 5 years ago
  1. While openpyxl allows loading spreadsheets from both file paths (str) and file-like objects, koala only supported file paths. The Spreadsheet constructor now checks the file parameter for a .read attribute, and if it has this attribute, assumes it is an open stream to a file rather than the file path string.
  2. Added Excel's two random (RAND and RANDBETWEEN) functions to the supported functions. Since python's random library is not thread-safe (threads would share the same seed), the code creates the random.Random() object directly so each call to the function has its own seed and is thread safe.

I also added two unit tests to test both ways of loading spreadsheets.

danielsjf commented 5 years ago

@doconix, great work, thanks for these changes. Could you also add a few tests for the rand functions? The file reading is fine.

doconix commented 5 years ago

Yes will do

doconix commented 5 years ago

OK got them in. I can't think of any "better" way to test these (tests are pretty simple). Let me know if you can think of further ways and I can implement.

vallettea commented 5 years ago

looks good, big thanks. last thing can you just increase the version number in package.json so i can push it to pipy

doconix commented 5 years ago

I updated the version number to 0.32 in setup.py. (I think that's what you meant... ;)

I also squashed the three commits into a single commit that goes with this PR so the git log is cleaner.

Let me know if you see anything else!

danielsjf commented 5 years ago

@doconix, it seems that similar changes ended up in the two pull requests. Could you disentangle them and clean the pulls? This now also has a speed refactor.

danielsjf commented 5 years ago

@doconix, after merging the other pull request, the issue is clearer.

Also, could you change the version to 0.0.34? Somehow, the version on Pypi is already 0.0.33 (granted, there seems to be a misalignment with the version on Github so let's fix that).

doconix commented 5 years ago

I somehow included the speedup refactor code in the previous PR. Apologies for that mixup. It was supposed to be a different branch/PR.

I've updated the remaining PR to have version 0.0.34, and the diff seems to be clear now (as you mention).

Thanks for looking at these PRs -- my company creates college-level online textbooks, and we are using koala to help grade embedded questions. Being able to convert Excel over to pure python is fantastic. These PRs are because we needed a few extra things to integrate it right, such as loading from S3 file streams rather than the server filesystem.

Let me know if you see anything else. Conan

danielsjf commented 5 years ago

@doconix, great to see all these uses and thanks a lot for your contribution.

@vallettea, could you update this version on Pypi?

vallettea commented 5 years ago

sorry for the delay, i updated on pipy