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 tabulate with prettytable #1749

Closed
opotowsky opened this issue Jun 26, 2024 Discussed in #1747 · 11 comments · Fixed by #1811
Closed

Replace tabulate with prettytable #1749

opotowsky opened this issue Jun 26, 2024 Discussed in #1747 · 11 comments · Fixed by #1811
Assignees
Labels
cleanup Code/comment cleanup: Low Priority good first issue Good for newcomers

Comments

@opotowsky
Copy link
Member

opotowsky commented Jun 26, 2024

Discussed in #1747

Originally posted by opotowsky June 25, 2024
tabulate is a dead project and prettytable is not. But it's pretty extensively used here and downstream with the runLog module.

NOTE: Because tabulate is a dead project, it is stopping us from moving to Python 3.11.

@opotowsky opotowsky added enhancement New feature or request cleanup Code/comment cleanup: Low Priority labels Jun 26, 2024
@john-science john-science added help wanted good first issue Good for newcomers and removed enhancement New feature or request labels Jun 26, 2024
@john-science
Copy link
Member

I have no particular love for tabulate, sure I'm game.

People might get touchy that you change the formatting of their log tables. But, that's not a big change, so I'm okay with it.

@jakehader
Copy link
Member

I'm not sure if replacing is necessary if tabulate functions as needed. What is the threshold for updating stale dependencies? I say if there is a clear bug or problem we have with tabulate then sure, but if it's just because the project is "dead" I'm not sure. Maybe there is something to be said about unmaintained projects and potentially vulnerabilities in open source packages with the more recent Linux / JavaScript supply chain attacks / hacks though.

See: xz, polyfill

@opotowsky
Copy link
Member Author

opotowsky commented Jul 5, 2024

@jakehader tabulate is blocking us from moving to python 3.11, so it's way above the threshold for "must remove". (Given the speedups we hope to see with switching)

Update: sorry, it runs with python 3.11, but it doesnt work over pip 22.2 with setup.py installs, which is blocking us from upgrading.

@jakehader
Copy link
Member

Thanks for the context. That wasn't in the discussion so I didn't know about an issue with pip or how it's blocking.

@opotowsky
Copy link
Member Author

Thanks for the context. That wasn't in the discussion so I didn't know about an issue with pip or how it's blocking.

True that! I think I must've offered the context in an offline chat before making the discussion. I'd need to retrace my steps to remember exactly how it's failing for us but it's something really lame

@jakehader
Copy link
Member

jakehader commented Jul 10, 2024

Sounds good - I'm fine with it if there is an issue with tabulate but if you can find the specific problem it would be good to provide that context in the ticket before updating so we can look back into the change / reason for the change.

I haven't used anything but tabulate before but if pretty-table is similar I'm down. Link - https://github.com/jazzband/prettytable

Article on comparison - https://amrinarosyd.medium.com/prettytable-vs-tabulate-which-should-you-use-e9054755f170#:~:text=PrettyTable%20allows%20you%20to%20customize,sort%20data%20by%20column%20values.&text=Tabulate%20offers%20several%20built%2Din,by%20modifying%20the%20table%20format.

@opotowsky
Copy link
Member Author

will do. Just need time to get back to my python311 work.

@devdanzin
Copy link

Would a fork of tabulate that includes bug fixes and supports newer Python versions help you, or is the move to prettytable the best solution? I ask because I and others have been thinking about forking into something like tabulate-ng.

@opotowsky
Copy link
Member Author

We want to move to a project that isn't deprecated and we can rely on updates being pushed to PyPI. Just helps keep things running over the longer term.

@john-science
Copy link
Member

Interestingly, I found the tabulate is only one Python file, and it's MIT licensed.

So I pulled the 2,800 line file out of that repo, and pulled it into an ARMI branch. It worked like a charm.

I also found I could reduce it to 1,500 lines, as there are a ton of features we don't want.

So... that might be an option.

@john-science john-science self-assigned this Aug 2, 2024
@opotowsky
Copy link
Member Author

I'm a fan! either ingest it or fork it. Don't matter to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code/comment cleanup: Low Priority good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants