Csvhelper: 12.3.2 > 13.0.0: cannot convert from StreamReader to CsvHelper.IParser

Created on 16 Jan 2020  路  27Comments  路  Source: JoshClose/CsvHelper

Describe the bug
Upgrading from 12.3.2 to 13.0.0 breaks this code:

2020-01-16_22-38-57

bug

Most helpful comment

It now requires CultureInfo to be passed in also.

I'm updating the change log right now.

All 27 comments

It now requires CultureInfo to be passed in also.

I'm updating the change log right now.

This is a breaking change, you should consider defaulting the CultureInfo if not provided (and keep those constructors).

Culture has been causing a lot of issues for people. It's now required so people will choose the correct culture for their file. It was previously using a default.

Any time there is a major version change, expect a breaking change in the API.

To elaborate on the fix (since I'm in the United States), I have provided an update of the following first example:

void Main()
{
    using (var reader = new StreamReader("path\\to\\file.csv"))
    using (var csv = new CsvReader(reader, System.Globalization.CultureInfo.CreateSpecificCulture("enUS")))
    {
        var records = csv.GetRecords<Foo>();
    }
}

One request to JoshClose: could this change be reflected in the original documentation? I don't mind breaking changes, but Google-Fu couldn't solve this problem, and only stumbling upon this issues thread did I realize this was a change.

You can use CurrentCulture and InvariantCulture too. The default used to be CurrentCulture.

I'm in the process of updating the docs. I've been having an issue building the docs otherwise they would be out there already.

Thanks Josh! Fantastic library, happy to use it. Thank you for your hard work.

No problem. I'll keep this open until I have the documentation deployed.

Agreed, thanks for all the work. your library is multiple orders of magnitude faster than code I had previously written by hand, and it goes without saying that since I discovered it, it has saved me many hours of manual parsing!

Awesome. Glad you find it useful.

I just discovered this issue when my build updated to the current revision. In the future, is there a elegant way to introduce breaking changes with some kind of DEPRECATION warning and also with a non-breaking overload that would warn to change to the new proper syntax so we get a chance to update our applications and not have to scramble to repair or down-rev packages. LOL.
It confused me at first. LOL. until I read the change log.
Thanks for maintaining this GREAT tool.

Your build updates to the latest NuGet package automatically? Is there a way to set it to only update on minor and revisions and exclude the major updates? Major updates will have breaking changes, minor and revision won't.

@JoshClose TBH, updating the changelog (https://joshclose.github.io/CsvHelper/change-log) at the same time as the package would be useful. That way we can see breaking changes to the API and not have to ask, or raise issues.

I'm wondering if you could catch the exception cannot convert from 'System.IO.StreamReader' to 'CsvHelper.IParser' and instead throw an exception that would guide users to the fact that they may need to include CultureInfo with CsvReader and CsvWriter? Otherwise I'm afraid there are going to be a lot of confused users.

Count me as one of those confused users, but once Josh Close gets his documentation issue resolved, it will be documented. Yes I would have loved this to be a default value so you can enter in your culture if you're having an issue, but any major version change most likely comes with some breaking changes. This one is pretty minor.

@DM2489 Sorry, I was having issues getting the build for the docs site to work. I intended on releasing that immediately after.

@AltruCoder They're build time errors though, right? I don't think you can catch anything on build time.

@TheMrAnderson The reason there is no default is because people were getting really confused by their files not parsing. It was defaulted to CultureInfo.CurrentCulture before. This would cause issues with formatting dates, etc, and with the delimiter. Now the user has to specifically state what they want and hopefully all those errors and confusion will go away.

I really really really wanted a way to not do this, but there have been just way too many people with confusion, and people that generally don't understand culture or why they would have to set it. This will put it in the forefront.

It's always recommended that you pass in a CultureInfo when doing any sort of formatting in .NET and I bet Microsoft would make it a required field if they had to do it again. I don't have the issue of needing to be backwards compatible to my 1.0 version though. ;)

The documentation site up and working... I think. https://joshclose.github.io/CsvHelper/change-log

I still need to update the examples. I was up until 2:00 am just trying to get it to build. I'll hopefully get that done some time today.

Examples are updated.

I'm going to use docfx to generate the API metadata to build API pages. Currently I have a custom one I built, but it doesn't catch everything. The API will be behind for a bit.

@JoshClose You are correct. The c# build time error message causes much of the confusion and could have been written more clearly.

@JoshClose I completely understand why you did this. I changed my code to pass in CurrentCulture and haven't had an issue with other formatters using CurrentCulture, but I'm also only running this on U.S. soil and U.S. generated documents. I see in your updated example, you have InvariantCulture. Do you recommend this over CurrentCulture?

If you're writing the files that you'll be reading back, then yes. It will work no matter where in the world your code runs. If you write a file using CurrentCulture in the US and then try to get the same file in Denmark using CurrentCulture, you'll have issues. The computer running in Denmark would need to specify en-US as the culture. If you want things formatted the way en-US formats, then by all means, use that. You just need to remember if the code runs in another culture, that it needs to specify that also. This is one of the big reasons why specifying your culture is important. It makes you think about what the content in your file is actually written as, or how you want the file to be written.

I was also a victim of this, added culture info and we are golden now. However, my qa department still wanted to do regression testing :)

@TheMrAnderson The reason there is no default is because people were getting really confused by their files not parsing. It was defaulted to CultureInfo.CurrentCulture before. This would cause issues with formatting dates, etc, and with the delimiter. Now the user has to specifically state what they want and hopefully all those errors and confusion will go away.

I really really really wanted a way to not do this, but there have been just way too many people with confusion, and people that generally don't understand culture or why they would have to set it. This will put it in the forefront.

It's always recommended that you pass in a CultureInfo when doing any sort of formatting in .NET and I bet Microsoft would make it a required field if they had to do it again. I don't have the issue of needing to be backwards compatible to my 1.0 version though. ;)

Is it worth a PR which defaults to the current culture if nothing is passed in?

No, this is intentional. It's to stop issues people are having with different cultures doing things differently. This makes it so people are explicit on how they want their formatting. Everyone expects something different as a default. Some people want CurrentCulture. Some people want InvariantCulture. Some cultures use different formatting than their CurrentCulture so neither of those options are good for them. There doesn't seem to be a default that can be used that will work for most people around the world. You can take a look at the discussion here and see some of the issues people were having. https://github.com/JoshClose/CsvHelper/issues/1203

All that you need is to pass the CultureInfo, sometimes it happened a problem when you create a new CultreInfo so just use the 'CurrentCulture' and it will work well.

var csvReader = new CsvReader(reader, CultureInfo.CurrentCulture);

we can achieve this thing using : var csv = new CsvReader(reader, System.Globalization.CultureInfo.CurrentCulture
Please see the code below , it will help you and will working fine
var engList = new List();
using (var reader = new StreamReader(path)) // file path and csv file
using (var csv = new CsvReader(reader, System.Globalization.CultureInfo.CurrentCulture))
{
engList = csv.GetRecords().Select(x=>new EngineModels() {
comp_name=x.comp_name,
model_name=x.model_name,
eng_size=x.eng_size,
eng_name=x.eng_name,
eng_description=x.eng_description,
title_h1=x.title_h1,
sub_title_h2=x.sub_title_h2,
page_title=x.page_title,
eng_metakeyword=x.eng_metakeyword,
eng_meta_description=x.eng_meta_description,
shown_main_listing=x.shown_main_listing
}).ToList();
}

Was this page helpful?
0 / 5 - 0 ratings