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

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

Merged
merged 14 commits into from
Jan 20, 2025

Conversation

Discolai
Copy link
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
Copy link
Member

izanhzh commented Jan 20, 2025

@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
Copy link
Member

izanhzh commented Jan 20, 2025

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

izanhzh commented Jan 20, 2025

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