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

GetColumnWidth: no need to explore the whole sheet #723

Merged
merged 2 commits into from
Feb 4, 2022
Merged

GetColumnWidth: no need to explore the whole sheet #723

merged 2 commits into from
Feb 4, 2022

Conversation

PatriceDargenton
Copy link
Contributor

GetColumnWidth can be speeded up for big sheets if we explore for example only the first 100 rows!

@tonyqus
Copy link
Member

tonyqus commented Jan 17, 2022

This can definitely speed up the process but this change cannot meet the needs of every NPOI users. How about making it a configurable argument in configuration file?

@PatriceDargenton
Copy link
Contributor Author

How about making it a configurable argument in configuration file?

But what if we want to use it and not use it in the same project?

How about an optional argument instead? (maxRows = 0 by default) Or another function GetColumnWidthFast?

@tonyqus
Copy link
Member

tonyqus commented Jan 18, 2022

I like maxRows = 0 by default idea. Can you change your PR?

@tonyqus tonyqus added this to the NPOI 2.5.6 milestone Jan 18, 2022
@PatriceDargenton
Copy link
Contributor Author

Done! (Do you prefer optional int parameter = 0 or nullable int parameter? I chose the first one but I don't know if it harmonizes with your code!)

@tonyqus
Copy link
Member

tonyqus commented Feb 4, 2022

@PatriceDargenton parameter = 0 is good

@tonyqus tonyqus self-requested a review February 4, 2022 06:14
@tonyqus tonyqus merged commit 14df981 into nissl-lab:master Feb 4, 2022
@tonyqus tonyqus modified the milestones: NPOI 2.6.0, NPOI 2.5.6 Apr 25, 2022
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.

2 participants