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

Add auto index column #106

Merged
merged 12 commits into from
Jun 6, 2021
Merged

Add auto index column #106

merged 12 commits into from
Jun 6, 2021

Conversation

erictzimas
Copy link
Contributor

@erictzimas erictzimas commented May 4, 2021

Fixes #90.

@codecov
Copy link

codecov bot commented May 4, 2021

Codecov Report

Merging #106 (b2ca525) into master (1ab53a5) will increase coverage by 0.27%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
+ Coverage   87.68%   87.96%   +0.27%     
==========================================
  Files           3        3              
  Lines        1535     1562      +27     
==========================================
+ Hits         1346     1374      +28     
+ Misses        189      188       -1     
Flag Coverage Δ
GHA_Ubuntu 87.96% <100.00%> (+0.27%) ⬆️
GHA_Windows 87.96% <100.00%> (+0.27%) ⬆️
GHA_macOS 87.96% <100.00%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/prettytable/prettytable.py 82.76% <100.00%> (+0.18%) ⬆️
tests/test_prettytable.py 100.00% <100.00%> (ø)

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 1ab53a5...b2ca525. Read the comment docs.

@hugovk hugovk mentioned this pull request May 4, 2021
@erictzimas
Copy link
Contributor Author

Should i add a test for this function? how should i handle those 5 failing checks?

@hugovk hugovk added enhancement New feature or request changelog: Added For new features labels May 4, 2021
@hugovk
Copy link
Member

hugovk commented May 4, 2021

Thanks for the PR!

Please could you add tests to tests/test_prettytable.py? Just let me know if you're not sure how to do that, and I can help out.

We can ignore the 3.10-dev failures for now, it's unrelated and caused by pytest not yet supporting 3.10 beta.

@erictzimas
Copy link
Contributor Author

I will work on it, thank you!

src/prettytable/prettytable.py Outdated Show resolved Hide resolved
tests/test_prettytable.py Outdated Show resolved Hide resolved
@hugovk
Copy link
Member

hugovk commented May 7, 2021

So this takes a table like this:

+-----------+------+------------+-----------------+
| City name | Area | Population | Annual Rainfall |
+-----------+------+------------+-----------------+
|  Adelaide | 1295 |  1158259   |      600.5      |
|  Brisbane | 5905 |  1857594   |      1146.4     |
|   Darwin  | 112  |   120900   |      1714.7     |
|   Hobart  | 1357 |   205556   |      619.5      |
|   Sydney  | 2058 |  4336374   |      1214.8     |
| Melbourne | 1566 |  3806092   |      646.9      |
|   Perth   | 5386 |  1554769   |      869.4      |
+-----------+------+------------+-----------------+

And adds an index to the right column:

+-----------+------+------------+-----------------+-------+
| City name | Area | Population | Annual Rainfall | Index |
+-----------+------+------------+-----------------+-------+
|  Adelaide | 1295 |  1158259   |      600.5      |   1   |
|  Brisbane | 5905 |  1857594   |      1146.4     |   2   |
|   Darwin  | 112  |   120900   |      1714.7     |   3   |
|   Hobart  | 1357 |   205556   |      619.5      |   4   |
|   Sydney  | 2058 |  4336374   |      1214.8     |   5   |
| Melbourne | 1566 |  3806092   |      646.9      |   6   |
|   Perth   | 5386 |  1554769   |      869.4      |   7   |
+-----------+------+------------+-----------------+-------+

However, @yanhuihang's example in #90 showed the index on the left column:

+-------+----------------------+----------------------+
| Index |          a           |          b           |
+-------+----------------------+----------------------+
|   1   |  0.8373683108488006  | -1.9662674211289342  |
|   2   |  0.8166690183400099  | -0.6827019085998253  |
|   3   | -0.34864565952912674 | -0.3828836864526866  |
|   4   |  -0.786195405989435  | -0.28278650269529015 |
|   5   | -0.7737543534146584  | 0.39198996847252016  |
|   6   |  1.2756295710689314  | 0.24976436160178775  |
|   7   |  -2.24766869070198   | -1.0958173510686795  |
|   8   | -0.2123610818363809  | -0.17583602522710734 |
|   9   | -0.9998924564529872  |  -0.78502826234026   |
|   10  |  -0.850991227375058  |  0.3267603399304381  |
+-------+----------------------+----------------------+

Would it be better on the left?

And should it be possible to set the column's field name to something else via a parameter? It can default to "Index".

@erictzimas
Copy link
Contributor Author

Taking everything you said into consideration, I think i will remove valign and align as it is not necessary to have a separate formatting in the index column, i will add a parameter that represents the name of the index column and i will try to add the column on the left as I agree, this how it is normally done. Thank you very much for the guidance.

@erictzimas
Copy link
Contributor Author

erictzimas commented May 8, 2021

I modified the function to add the column on the left, and added the fieldname parameter, i did not remove valign and align since it does not seem to work if i do not specify this attributes when adding a new column as in add_column()

def add_column(self, fieldname, column, align="c", valign="t"):

        """Add a column to the table.

        Arguments:

        fieldname - name of the field to contain the new column of data
        column - column of data, should be a list with as many elements as the
        table has rows
        align - desired alignment for this column - "l" for left, "c" for centre and
            "r" for right
        valign - desired vertical alignment for new columns - "t" for top,
            "m" for middle and "b" for bottom"""

        if len(self._rows) in (0, len(column)):
            self._validate_align(align)
            self._validate_valign(valign)
            self._field_names.append(fieldname)
            self._align[fieldname] = align
            self._valign[fieldname] = valign
            for i in range(0, len(column)):
                if len(self._rows) < i + 1:
                    self._rows.append([])
                self._rows[i].append(column[i])
        else:
            raise Exception(
                f"Column length {len(column)} does not match number of rows "
                f"{len(self._rows)}"
            )

If that is ok please inform me so i can create a PR that also covers the _valign parameter, thank you.

src/prettytable/prettytable.py Outdated Show resolved Hide resolved
@hugovk hugovk changed the title Add auto_index function as per #90 Add auto index column Jun 6, 2021
@hugovk hugovk merged commit 16b4a3b into jazzband:master Jun 6, 2021
@hugovk
Copy link
Member

hugovk commented Jun 6, 2021

Thank you very much!

@mailsanchu
Copy link

Is this index broken if you apply sort?

@hugovk
Copy link
Member

hugovk commented Jun 17, 2021

It adds a column to the data, so if you sort after adding, yes. So it's best sort first and add the index last.

@mailsanchu
Copy link

Cool Is it possible to formatted index like 001, 002 etc ?

@hugovk
Copy link
Member

hugovk commented Jun 18, 2021

No.

@mailsanchu
Copy link

No worries

@hugovk
Copy link
Member

hugovk commented Jun 18, 2021

You can always manually add a column with whatever data you want, including numbers formatted however you like.

@mailsanchu
Copy link

mailsanchu commented Jun 18, 2021

Yes I have done exactly that. Thanks

my_list.sort(key=lambda a: a[1])
for idx, val in enumerate(my_list):
    val.insert(0, f"{(idx + 1):0>3}")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: Added For new features enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest: auto index column
3 participants