Skip to content

Conversation

sh-andriy
Copy link

…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
Copy link

coveralls commented Aug 5, 2025

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.

Copy link
Contributor

@cslzchen cslzchen 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 overall with a few questions ⭐

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
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?

…and display overflow banner if table exceeds MAX_SIZE limit
Copy link
Contributor

@cslzchen cslzchen 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 with a non-blocking suggestion :)

@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
3 participants