Add support to Import/Export table to/from dataframe #126

Agent-Hellboy · 2022-04-01T07:46:22Z

fixes #97

codecov-commenter · 2022-04-01T12:23:05Z

Codecov Report

Merging #126 (6068d3d) into master (6996928) will decrease coverage by 0.03%.
The diff coverage is 80.95%.

@@            Coverage Diff             @@
##           master     #126      +/-   ##
==========================================
- Coverage   83.09%   83.06%   -0.04%     
==========================================
  Files          11       11              
  Lines        1384     1405      +21     
  Branches      244      250       +6     
==========================================
+ Hits         1150     1167      +17     
- Misses        156      159       +3     
- Partials       78       79       +1

Impacted Files	Coverage Δ
beautifultable/beautifultable.py	`83.00% <80.95%> (-0.10%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6996928...6068d3d. Read the comment docs.

Agent-Hellboy · 2022-04-01T12:26:55Z

Sorry, for adding multiple commits, You can squash while merging. I tried but I was not able to squash these. I am not able to figure out why the formatting is failing.

beautifultable/beautifultable.py

pri22296 · 2022-04-01T14:33:22Z

beautifultable/beautifultable.py

@@ -30,6 +30,9 @@
 import copy
 import csv
 import warnings
+from numpy import r_
+
+import pandas as pd


Can we make pandas an optional dependency? so ideally just raise an exception inside the to_df, from_df methods if pandas is not installed. do make them required dependencies for the tests though. I think setup.py has fields for this scenario.

Sure, will look into this.

beautifultable/beautifultable.py

test.py

pri22296 · 2022-04-01T14:41:26Z

test.py

+    def test_df_import(self):
+        table = BeautifulTable()
+        table = table.from_df(self.df)
+        self.assertEqual(self.table.rows.header, self.df.index)


we should also compare the cell values, not just the headers. also maybe add other unit tests to check the behavior in case headers are missing.

Let's not use the _data attribute in the unit tests. private variable so only meant to be used from internal modules.
[list(row) for row in list(self.table._data)] => [list(row) for row in table.rows]

Yes, I have changed this in beautifultable.py also. do you know I used [list(row) for row in table.rows] while adding asdict API, don't know why I used _data here.
No probs, I am thinking of picking the row-span, column-span, and transpose features there I will get some good exposure to the design.

spans might be tricky. needs some design before any implementations. I did start with something but found some gaps in the approach hence had to abandon the idea. but yeah, would love to see if you come up with an approach on how to implement these on their respective threads.

pri22296 · 2022-04-01T14:45:24Z

@Agent-Hellboy added few comments. btw the formatting tests are failing because you haven't run black on the source code. It's not documented properly that it should be done. apologies for that.

https://github.com/psf/black

Agent-Hellboy · 2022-04-02T15:11:23Z

Thanks for the quick review Priyam
I don't know why formatting is failing for test.py. I ran black and the failure message is suggesting that everything is okay.

 proshan@proshan-ThinkPad-L480  ~/ADD_FEATURE_BEAUTIFULTABLE/beautifultable   master  black test.py 
All done! ✨ 🍰 ✨
1 file left unchanged.

Am I missing some flag while running black test.py?
one more question When are you planning to add the type annotation?

pri22296 · 2022-04-02T16:07:30Z

@Agent-Hellboy Looks like formatting on master is also failing. I did test locally with the latest version of black and I do see the errors. Make sure you're using python3 to install and run black.

so somethink like

python3 -m pip install black
python3 -m black test.py

About annotations, can you open an issue for that. Better to track that in a separate thread.

Agent-Hellboy · 2022-04-03T17:21:54Z

@pri22296 I got why formatting was failing. str literals are Unicode instead of bytes in 3.9, so I think we don't need to use u"" anymore. I was using black with 3.8. when I used the latest python nightly 3.11dev, it formatted test.py correctly.
Now CI is failing with PyPy. It is failing because of actions/setup-python#244
I am adding a patch-set with required changes

Agent-Hellboy · 2022-04-03T18:42:16Z

still failing for PyPy, Here people are saying in comments that we can't build pandas in pypy without a hack
https://stackoverflow.com/questions/39500612/any-way-to-install-pandas-with-pypy

pri22296 · 2022-04-04T18:48:06Z

@Agent-Hellboy For some reason, when using Pypy, pandas is raising an import warning, which leads to the build failures. for now, you can replace the following

beautifultable/.github/workflows/build.yml

Line 80 in 6996928

python -W error test.py

with

python test.py

This should fix the build errors.

beautifultable/beautifultable.py

test.py

beautifultable/beautifultable.py

.github/workflows/build.yml

Agent-Hellboy · 2022-04-19T14:08:34Z

@pri22296 are you waiting for me to increase the code coverage, or you are busy in your work?

pri22296 · 2022-04-29T16:34:10Z

@pri22296 are you waiting for me to increase the code coverage, or you are busy in your work?

@Agent-Hellboy apologies for the late reply. I can see that there are still few unresolved comments. could you please address those so we can go ahead with this change.

Agent-Hellboy · 2022-05-02T10:38:07Z

@pri22296 are you waiting for me to increase the code coverage, or you are busy in your work?

@Agent-Hellboy apologies for the late reply. I can see that there are still few unresolved comments. could you please address those so we can go ahead with this change.

are you talking about these two? I am waiting for your response on these two comments
#126 (comment)
#126 (comment)

pri22296 · 2022-05-03T04:45:51Z

@Agent-Hellboy That's so weird. I don't see any comment from you in that thread. Can you maybe paste your comments here? must be some issue on github's end.

Agent-Hellboy · 2022-05-04T09:21:13Z

Have we considered the case where some values in the headers are None and some aren't? what happens then? pandas wouldn't allow duplicate indexes so would it raise an exception? I'm not sure if it even allows None as an index value.

Actually, they are allowing these things but later giving some intelligent response by taking some decision
example
it is allowing the duplicate index

>>> df.rank()
   rank  rank  rank
0   2.0   1.0   1.0
1   1.0   2.0   2.0
>>> df.to_dict()
<stdin>:1: UserWarning: DataFrame columns are not unique, some columns will be omitted.
{'rank': {0: 'boy', 1: 'girl'}}

It is allowing None as the header also

>>> df.columns
Index([None, 'p', 'rank'], dtype='object')

in case of reading from a dataframe, let's just use the indexes as row headers as is. so, by default, let that be [0,1,2,...]. If the user wants to hide them, they'll add an explicit call table.rows.headers = None after importing the dataframe.

Done

Agent-Hellboy added 8 commits April 1, 2022 13:10

Add support to Import/Export table to/from dataframe

604ab0f

Add support to Import/Export table to/from dataframe

06048e4

Merge branch 'master' of https://github.com/Agent-Hellboy/beautifultable

04c927a

Merge branch 'master' of https://github.com/Agent-Hellboy/beautifultable

4639e7a

Merge branch 'master' of https://github.com/Agent-Hellboy/beautifultable

8a8f925

Add support to Import/Export table to/from dataframe

b811b19

Add support to Import/Export table to/from dataframe

097136b

Merge branch 'master' of https://github.com/Agent-Hellboy/beautifultable

9a3ae7a

pri22296 requested changes Apr 1, 2022

View reviewed changes

Agent-Hellboy added 3 commits April 1, 2022 22:01

Add support to Import/Export table to/from dataframe

f577d79

Add support to Import/Export table to/from dataframe

5aee800

Add support to Import/Export table to/from dataframe

f933058

Add support to Import/Export table to/from dataframe

200c380

Agent-Hellboy added 2 commits April 3, 2022 22:58

Fix version in PyPy to avoid build failure

c55c4e8

Fix version in PyPy to avoid build failure

4d506b0

Fix unittest Failures

14539fb

pri22296 requested changes Apr 4, 2022

View reviewed changes

Add support to Import/Export table to/from dataframe

6068d3d

pri22296 merged commit df378e8 into pri22296:master May 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to Import/Export table to/from dataframe #126

Add support to Import/Export table to/from dataframe #126

Agent-Hellboy commented Apr 1, 2022

codecov-commenter commented Apr 1, 2022 •

edited

Loading

Agent-Hellboy commented Apr 1, 2022

pri22296 Apr 1, 2022

Agent-Hellboy Apr 1, 2022

pri22296 Apr 1, 2022

Agent-Hellboy Apr 1, 2022

pri22296 Apr 2, 2022 •

edited

Loading

Agent-Hellboy Apr 2, 2022 •

edited

Loading

pri22296 Apr 2, 2022

Agent-Hellboy Apr 3, 2022

pri22296 commented Apr 1, 2022

Agent-Hellboy commented Apr 2, 2022

pri22296 commented Apr 2, 2022

Agent-Hellboy commented Apr 3, 2022 •

edited

Loading

Agent-Hellboy commented Apr 3, 2022

pri22296 commented Apr 4, 2022

Agent-Hellboy commented Apr 19, 2022 •

edited

Loading

pri22296 commented Apr 29, 2022 •

edited

Loading

Agent-Hellboy commented May 2, 2022 •

edited

Loading

pri22296 commented May 3, 2022

Agent-Hellboy commented May 4, 2022 •

edited

Loading

Add support to Import/Export table to/from dataframe #126

Add support to Import/Export table to/from dataframe #126

Conversation

Agent-Hellboy commented Apr 1, 2022

codecov-commenter commented Apr 1, 2022 • edited Loading

Codecov Report

Agent-Hellboy commented Apr 1, 2022

pri22296 Apr 1, 2022

Choose a reason for hiding this comment

Agent-Hellboy Apr 1, 2022

Choose a reason for hiding this comment

pri22296 Apr 1, 2022

Choose a reason for hiding this comment

Agent-Hellboy Apr 1, 2022

Choose a reason for hiding this comment

pri22296 Apr 2, 2022 • edited Loading

Choose a reason for hiding this comment

Agent-Hellboy Apr 2, 2022 • edited Loading

Choose a reason for hiding this comment

pri22296 Apr 2, 2022

Choose a reason for hiding this comment

Agent-Hellboy Apr 3, 2022

Choose a reason for hiding this comment

pri22296 commented Apr 1, 2022

Agent-Hellboy commented Apr 2, 2022

pri22296 commented Apr 2, 2022

Agent-Hellboy commented Apr 3, 2022 • edited Loading

Agent-Hellboy commented Apr 3, 2022

pri22296 commented Apr 4, 2022

Agent-Hellboy commented Apr 19, 2022 • edited Loading

pri22296 commented Apr 29, 2022 • edited Loading

Agent-Hellboy commented May 2, 2022 • edited Loading

pri22296 commented May 3, 2022

Agent-Hellboy commented May 4, 2022 • edited Loading

codecov-commenter commented Apr 1, 2022 •

edited

Loading

pri22296 Apr 2, 2022 •

edited

Loading

Agent-Hellboy Apr 2, 2022 •

edited

Loading

Agent-Hellboy commented Apr 3, 2022 •

edited

Loading

Agent-Hellboy commented Apr 19, 2022 •

edited

Loading

pri22296 commented Apr 29, 2022 •

edited

Loading

Agent-Hellboy commented May 2, 2022 •

edited

Loading

Agent-Hellboy commented May 4, 2022 •

edited

Loading