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

Fix decimal parsing issue for scientific notation in DbfNumericDecimalField (#57) #58

Merged
merged 2 commits into from
Sep 7, 2024

Conversation

DiegoNetoMartins
Copy link
Contributor

@DiegoNetoMartins DiegoNetoMartins commented Sep 5, 2024

This PR addresses the issue with decimal parsing in scientific notation described in issue #57 .

Fixes #57

…lField

The `DbfNumericDecimalField` class in the `NetTopologySuite.IO.Esri` library failed to correctly parse numeric fields in scientific notation (e.g., "9.095630227029324e-05") due to the use of `decimal.Parse` without `NumberStyles.Float`. This resulted in a format exception.

Updated the parsing logic to use `NumberStyles.Float` with `decimal.Parse` to handle scientific notation correctly.

Affected class: `DbfNumericDecimalField`
@CLAassistant
Copy link

CLAassistant commented Sep 5, 2024

CLA assistant check
All committers have signed the CLA.

@KubaSzostak
Copy link
Member

Hi @DiegoNetoMartins, thank you for identifying this issue and suggesting a fix. Your proposed modification looks solid. To ensure comprehensive coverage, could you please add a unit test to validate this change? You might find it helpful to follow the existing test for the Issue049, which also relates to the DbfNumericDecimalField. Looking forward to your update!

@DiegoNetoMartins
Copy link
Contributor Author

Hi @DiegoNetoMartins, thank you for identifying this issue and suggesting a fix. Your proposed modification looks solid. To ensure comprehensive coverage, could you please add a unit test to validate this change? You might find it helpful to follow the existing test for the Issue049, which also relates to the DbfNumericDecimalField. Looking forward to your update!

Hi @KubaSzostak ,

Thank you for your feedback. I have added a unit test to validate the change related to DbfNumericDecimalField. You can find the changes in the following commit: d1f50ed

Please let me know if there are any further adjustments needed.

Looking forward to your review!

Best regards,
Diego

@KubaSzostak KubaSzostak merged commit 9799852 into NetTopologySuite:develop Sep 7, 2024
6 checks passed
@KubaSzostak
Copy link
Member

Thank you @DiegoNetoMartins for fixing the issue and and for adding the unit test. The changes look great, and the test case covers the modification well. Looking forward to any future collaborations!

@DiegoNetoMartins
Copy link
Contributor Author

Thank you @DiegoNetoMartins for fixing the issue and and for adding the unit test. The changes look great, and the test case covers the modification well. Looking forward to any future collaborations!

Hi @KubaSzostak, thanks for the feedback! Could you let me know when these updates will be included in the NuGet package?

@KubaSzostak
Copy link
Member

@FObermaier, can we push the latest fixes to the NuGet repository?

@FObermaier
Copy link
Member

@KubaSzostak, done.

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.

Error processing numeric fields in scientific notation with DbfNumericDecimalField
4 participants