Skip to content

Generalize excel writing with a common write adapter and implement writing IAsyncEnumerable - #712

Merged
shps951023 merged 14 commits into
mini-software:masterfrom
Discolai:write-adapter-interface
Jan 20, 2025
Merged

Generalize excel writing with a common write adapter and implement writing IAsyncEnumerable#712
shps951023 merged 14 commits into
mini-software:masterfrom
Discolai:write-adapter-interface

Conversation

@Discolai

Copy link
Copy Markdown
Contributor

I have done some work previously with the writing logic in MiniExcel and I always found it cumbersome to duplicate it across different datasource implementations .

This PR adds two new interfaces IMiniExcelWriteAdapter and IAsyncMiniExcelWriteAdapter to generalize data fetching for writing to excel or csv. Which makes is much easier to add support for new datasources as well as adding new functionality or fixing bugs.

I also added support for writing IAsyncEnumerable to both excel and csv.

#706 added support for writing asynchronously with the IMiniExcelDataReader. I moved this implementation into MiniExcelDataReaderWriteAdapter which uses IAsyncEnumerable internally. This makes it incompatible with dotnet 4.5. @izanhzh

Another change which might be breaking is that writing an empty collection will no longer produce 1 row, but 0.

@izanhzh

This comment was marked as resolved.

@izanhzh

izanhzh commented Jan 20, 2025

Copy link
Copy Markdown
Member

@Discolai I change some of the code, so that there are no more break change

Another change which might be breaking is that writing an empty collection will no longer produce 1 row, but 0.

@shps951023 Can we mark FastMode as obsolete, why do we still need to keep ZipArchiveMode.Create, ZipArchiveMode.Update also support the creation of new compression packages, If ZipArchiveMode.Update is used uniformly, we do not need to know the count of columns to write dimension in advance, there are same data types that do not know this value in advance, such as IDataReader , which does not currently output dimension

@izanhzh
izanhzh requested a review from shps951023 January 20, 2025 06:44
@izanhzh

izanhzh commented Jan 20, 2025

Copy link
Copy Markdown
Member

image
Previously, if it was not FastMode, the IEnumerable would be converted to a List. In order to write dimension, it would no longer be able to write dimension after this PR

@Discolai

Copy link
Copy Markdown
Contributor Author

image Previously, if it was not FastMode, the IEnumerable would be converted to a List. In order to write dimension, it would no longer be able to write dimension after this PR

I realized the same as you did for IDataReader. So I assumed that dimensions is not required, but rather a nice to have. Do you have any input @shps951023 ?

@izanhzh

izanhzh commented Jan 20, 2025

Copy link
Copy Markdown
Member

image Previously, if it was not FastMode, the IEnumerable would be converted to a List. In order to write dimension, it would no longer be able to write dimension after this PR

I realized the same as you did for IDataReader. So I assumed that dimensions is not required, but rather a nice to have. Do you have any input @shps951023 ?

Ha ~, IDataReader is not added by me, I just extended asynchronous support, I have found some information that if dimensions are not used, it may be slow to recalculate large files when opening them, it is not necessary. I think we can use all the FastMode way, I'm not sure what other special features Not-FastMode has

@Discolai

Copy link
Copy Markdown
Contributor Author

image Previously, if it was not FastMode, the IEnumerable would be converted to a List. In order to write dimension, it would no longer be able to write dimension after this PR

I realized the same as you did for IDataReader. So I assumed that dimensions is not required, but rather a nice to have. Do you have any input @shps951023 ?

Ha ~, IDataReader is not added by me, I just extended asynchronous support, I have found some information that if dimensions are not used, it may be slow to recalculate large files when opening them, it is not necessary. I think we can use all the FastMode way, I'm not sure what other special features Not-FastMode has

Yes I agree that we can remove/deprecate FastMode unless there is some specific reason to keep it around

@shps951023

Copy link
Copy Markdown
Member

I have done some work previously with the writing logic in MiniExcel and I always found it cumbersome to duplicate it across different datasource implementations .

Couldn't agree more, current work loading is hard between async and non-async.

@shps951023

Copy link
Copy Markdown
Member

@shps951023 Can we mark FastMode as obsolete, why do we still need to keep ZipArchiveMode.Create, ZipArchiveMode.Update also support the creation of new compression packages, If ZipArchiveMode.Update is used uniformly, we do not need to know the count of columns to write dimension in advance, there are same data types that do not know this value in advance, such as IDataReader , which does not currently output dimension

@izanhzh Some people would like to use higher source to get better performance, for me, I only use ZipArchiveMode.Create by miniexcel. we can develop miniexcel new functions only for ZipArchiveMode.Create. Becuase ZipArchiveMode.Update is not the core of miniexcel :) .

@shps951023
shps951023 merged commit 9e5fea6 into mini-software:master Jan 20, 2025
pull Bot pushed a commit to zilo555/MiniExcel that referenced this pull request Aug 29, 2026
…iting IAsyncEnumerable (mini-software#712)

* Generalize excel writing with a common write adapter

* Remove testcase for single row in empty table

* Implement sync IExcelWriteAdapter

* Remove testcase for single row in empty table

* Add back support for writing IMiniExcelDataReader

* Add support for writing AsyncEnumerable

* Add cancellation support

* Add MiniExcelWriteAdapterFactory

* Implement write adapters for csv writing

* cancel break change

* fix build bug

* fix get props bug

* cancel some adjust

* clean code

---------

Co-authored-by: izanhzh <hzh990094740@outlook.com>
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