Closed BD103 closed 2 years ago
Merging #140 (97cccf0) into master (03f6ce9) will increase coverage by
0.49%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #140 +/- ##
==========================================
+ Coverage 92.92% 93.41% +0.49%
==========================================
Files 3 5 +2
Lines 1978 2126 +148
==========================================
+ Hits 1838 1986 +148
Misses 140 140
Flag | Coverage Δ | |
---|---|---|
GHA_Ubuntu | 93.41% <100.00%> (+0.49%) |
:arrow_up: |
GHA_Windows | 93.27% <97.05%> (+0.35%) |
:arrow_up: |
GHA_macOS | 93.41% <100.00%> (+0.49%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
src/prettytable/colortable.py | 100.00% <100.00%> (ø) |
|
tests/test_colortable.py | 100.00% <100.00%> (ø) |
|
tests/test_prettytable.py | 100.00% <0.00%> (ø) |
|
src/prettytable/prettytable.py | 89.48% <0.00%> (+0.20%) |
:arrow_up: |
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 03f6ce9...97cccf0. Read the comment docs.
I'll convert this to draft for now, and we can put it as "ready" when you're ready :)
I still have some more tests to add, but I'm getting an error. It says that it AttributeError: module 'pytest' has no attribute 'lazy_fixture'
. I have to test my code by ignoring the main test script, which probably isn't good.
Also, should I test if Colorama is initialized?
Thanks, -BD103
I still have some more tests to add, but I'm getting an error. It says that it
AttributeError: module 'pytest' has no attribute 'lazy_fixture'
.
You need to install pytest-lazy-fixture for this.
(It's part of python -m pip install -e ".[tests]"
)
Thank you! I'll get this done and report back when finished.
I have completed my task list, so this PR is ready for review. There is probably room for improvement with the tests, but for now, it is good. :)
Thank you again for the PR!
Is it so that if people want to use colourful table, they need to use the ColorTable
subclass instead of the usual PrettyTable
?
Would it be possible to refactor it so that people could continue to use PrettyTable
and instead set the desired theme?
It would be possible to integrate theming into the PrettyTable
class, but I would caution against it. ColorTable
adds some extra processing and may cause unintended side effects. For instance:
sys.stdout
and may break code related to that\x1b[0m
to the end of each table, which will break all testsVERTICAL_COLOR + VERTICAL_CHAR + RESET + DEFAULT_COLOR
, instead of VERTICAL_CHAR
. (\x1b[31m|\x1b[0m\x1b[32m
instead of |
.)While it would be very convenient in transitioning to colorful tables, it seems best to rename a class here or there.
There we go, the readme is updated and unused fixture removed. :)
Thank you!
Great, I'm so glad we could get this merged! :D
Hello! I am back again. I felt bad after leaving the original PR stale, so I came back and redid it.
(Better late than never!)
I tried to follow all the programming standards with Pre-commit and the likewise. Some of my goals for this are:
Thank you for putting up with me, ~BD103
(Fixes #86)