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

ENH: Simplify table2string implementation #1403

Merged
merged 1 commit into from
Jan 12, 2025

Conversation

effigies
Copy link
Member

The previous table2string implementation reimplemented Python formatting, calculating spaces for aligning left, right or center. This patch just translates our mini-language into the Python mini-language, and updates the tests.

Previously, when centering required adding an odd number of spaces, we added the extra to the left, while Python adds to the right. This difference does not seem worth preserving.

This also avoids allocating a numpy array in order to transpose by using the zip(*list) trick. Finally, we concatenate the strings in single shots, which separates the entry formatting from the table formatting and makes the function easier to understand, IMO.

The previous table2string implementation reimplemented Python
formatting, calculating spaces for aligning left, right or center. This
patch just translates our mini-language into the Python mini-language,
and updates the tests.

Previously, when centering required adding an odd number of spaces, we
added the extra to the left, while Python adds to the right.
This difference does not seem worth preserving.

This also avoids allocating a numpy array in order to transpose by using
the `zip(*list)` trick.
Copy link

codecov bot commented Jan 12, 2025

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.38%. Comparing base (59733ff) to head (99c88f1).
Report is 34 commits behind head on master.

Files with missing lines Patch % Lines
nibabel/cmdline/utils.py 84.61% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1403      +/-   ##
==========================================
+ Coverage   95.37%   95.38%   +0.01%     
==========================================
  Files         207      207              
  Lines       29672    29659      -13     
  Branches     3018     3014       -4     
==========================================
- Hits        28300    28291       -9     
+ Misses        934      932       -2     
+ Partials      438      436       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@effigies effigies changed the title rf(cmdline): Simplify table2string implementation ENH: Simplify table2string implementation Jan 12, 2025
@effigies effigies merged commit b6ca0d6 into nipy:master Jan 12, 2025
39 of 41 checks passed
@effigies effigies deleted the enh/simplify-table2string branch January 12, 2025 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant