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

Replace the use of ptable with prettytable #1584

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

dvzrv
Copy link
Contributor

@dvzrv dvzrv commented Feb 9, 2022

{README.rst,tools/*}:
Replace ptable with prettytable >= 2.0.0.

SoftLayer/CLI/formatting.py:
Only import prettytable.

Fixes #1583

@dvzrv dvzrv force-pushed the switch_to_prettytable branch from e74d5f0 to 6f0bf99 Compare February 9, 2022 23:26
@allmightyspiff allmightyspiff added the Core Issues dealing with core functionality label Feb 10, 2022
Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to update setup.py to add this library as well. That should at least get the tests to work.

My only concern is that anyone with the PTable library installed will run into import errors, which is the problem I ran into last time I tried to switch over to 100% PrettyTable (hence try/except in formatting.py).

Thanks for the pull request though, provided this doesn't cause users upgrade issues this change should be good.

{README.rst,tools/*}:
Replace ptable with prettytable >= 2.0.0.

SoftLayer/CLI/formatting.py:
Only consider the import of prettytable.
@dvzrv dvzrv force-pushed the switch_to_prettytable branch from 6f0bf99 to bc42a74 Compare February 10, 2022 08:26
@dvzrv
Copy link
Contributor Author

dvzrv commented Feb 10, 2022

I think you need to update setup.py to add this library as well. That should at least get the tests to work.

Ah, right! There are a bit too many places where the dependencies are set 😆
Thanks for the hint!

My only concern is that anyone with the PTable library installed will run into import errors, which is the problem I ran into last time I tried to switch over to 100% PrettyTable (hence try/except in formatting.py).

I guess ptable should be dropped from all systems. There has not been any development effort in years.

Thanks for the pull request though, provided this doesn't cause users upgrade issues this change should be good.

Thanks! I hope it works out as intended, as it allows us on Arch Linux to drop the python-ptable package (which conflicts with python-prettytable and is only required for this project)

Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks.
Tested upgrading from 5.9.9 to this version ran without issue, so no worries there either.

@allmightyspiff allmightyspiff merged commit bfcbc02 into softlayer:master Feb 11, 2022
@allmightyspiff allmightyspiff mentioned this pull request Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issues dealing with core functionality
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Ptable is abandoned and can be replaced with prettytable
2 participants