Refactor tbl data #9
Conversation
|
I hope it's okay I lumped in a doc example fix, and added a test of fmt_number! |
|
@rich-iannone I think this is ready--happy to walk through while pairing tomorrow! I think it might be easier to merge this in (happy to add any tweaks, clarify anything), then implement / add tests for some of things in examples-qmd. I added tests of the dataframe generic functions, and we should know whether we need to tweak once we test some of the functions that call them |
| @@ -29,7 +29,7 @@ jobs: | |||
| if [ -f requirements-dev.txt ]; then pip install -r requirements-dev.txt; fi | |||
| - name: Install | |||
| run: | | |||
| make install | |||
There was a problem hiding this comment.
I wasn't sure what to add to requirements-dev.txt, so switched to installing the package with dev dependencies. Happy to tweak!
| df = pd.DataFrame({"x": [1.234, 2.345], "y": [3.456, 4.567]}) | ||
| gt = GT(df).fmt_number(columns="x", decimals=2) | ||
|
|
||
| # TODO: is 2.35 below the intended result? |
There was a problem hiding this comment.
I wasn't sure if gt deliberately rounds, so that 2.345 rounded is 2.35
There was a problem hiding this comment.
I think if it doesn't through the formatter we should deliberately pre-round before formatting (then we could expose the rounding method too).
|
This looks great! Just going to review, approve, and you can merge in whenever! |
| # TODO: I think that this is very inefficient with polars, so | ||
| # we could either accumulate results and set them per column, or | ||
| # could always use a pandas DataFrame inside Body? | ||
| _set_cell(self.body, row, col, result) |
There was a problem hiding this comment.
Setting individual cells is inefficient in polars (I think it sets a whole column copy), and it looks like they're not planning on changing that (since they're more analytic / column oriented).
Shouldn't be an issue for prototyping, but wanted to flag
|
Ahhh yeeeeaaahhhhhh!!! |
This PR works toward #7 by..
_tbl_data.n_rows)I noticed pandas DataFrames are created in some places (e.g. Spanners). It might be useful for us to keep the pandas DataFrame as frame used within some of these classes (at least for now?), just to make life simpler while getting all the different features out.
Happy to make any changes, discuss anything!