Skip to content

[ENG-8489] | enforce MAX_SIZE on .xls rendering just like was on .xlsx …#397

Merged
cslzchen merged 2 commits into
CenterForOpenScience:feature/buff-wormsfrom
sh-andriy:fix/ENG-8489
Aug 5, 2025
Merged

[ENG-8489] | enforce MAX_SIZE on .xls rendering just like was on .xlsx …#397
cslzchen merged 2 commits into
CenterForOpenScience:feature/buff-wormsfrom
sh-andriy:fix/ENG-8489

Conversation

@sh-andriy

Copy link
Copy Markdown
Contributor

…and display overflow banner if table exceeds MAX_SIZE limit

Ticket

Purpose

Changes

Side effects

QA Notes

Deployment Notes

…and display overflow banner if table exceeds MAX_SIZE limit
@coveralls

coveralls commented Aug 5, 2025

Copy link
Copy Markdown

Coverage Status

coverage: 68.586% (+0.05%) from 68.538%
when pulling 6e29004 on sh-andriy:fix/ENG-8489
into 66b1859 on CenterForOpenScience:feature/buff-worms.

@cslzchen cslzchen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good overall with a few questions ⭐

Comment thread mfr/extensions/tabular/utilities.py
Comment thread mfr/extensions/tabular/utilities.py Outdated
if getattr(ws, 'max_row', None) is None or getattr(ws, 'max_column', None) is None:
raise CorruptedError

ncols = getattr(ws, "max_column", 0) or 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need the or 0 part?

Comment thread mfr/extensions/tabular/utilities.py
Comment thread mfr/extensions/tabular/utilities.py
Comment thread mfr/extensions/tabular/utilities.py
…and display overflow banner if table exceeds MAX_SIZE limit

@cslzchen cslzchen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good with a non-blocking suggestion :)

Comment thread mfr/extensions/tabular/utilities.py
Comment thread mfr/extensions/tabular/utilities.py
Comment thread mfr/extensions/tabular/utilities.py
Comment thread mfr/extensions/tabular/utilities.py
@cslzchen
cslzchen merged commit 231f5be into CenterForOpenScience:feature/buff-worms Aug 5, 2025
2 checks passed
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.

3 participants