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

[release/8.0-staging] Fix an issue with complex number parsing #104499

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jul 6, 2024

Backport of #104388 to release/8.0-staging

/cc @tannergooding

Customer Impact

  • Customer reported
  • Found internally

As per #104421, developers attempting to parse a string representing a Complex number would see an error for otherwise valid inputs.

Regression

  • Yes
  • No

This was a net new API and has been broken since it was introduced.

Testing

Additional testing covering a broader range of inputs was added.

Risk

Low. This is a net new API that has overall low usage and is simply allowing valid inputs to parse as expected instead of throwing or reporting that it could not be parsed.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

@tannergooding tannergooding added the Servicing-consider Issue for next servicing release review label Jul 6, 2024
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jul 9, 2024
@leecow leecow added this to the 8.0.8 milestone Jul 9, 2024
@ericstj
Copy link
Member

ericstj commented Jul 9, 2024

@tannergooding is there any reverse compat concern here, or is this only impacting an exception case and unblocking that case?

I don't think so, but I wanted to double check.

@tannergooding
Copy link
Member

@ericstj someone could theoretically be depending on the TryParse path returning false here, but given its a new API the chance of that is overall low. It's also unexpected, hence why the customer reported the issue as they expected the input produced by ToString() to have successfully been parsed back out.

@ericstj
Copy link
Member

ericstj commented Jul 10, 2024

they expected the input produced by ToString() to have successfully been parsed back out.

Yep and this doesn't change that output at all (which could be persisted) and just makes it parse correctly. Seems safe from a compat perspective.

@tannergooding tannergooding merged commit 3d0baeb into release/8.0-staging Jul 10, 2024
107 of 113 checks passed
@tannergooding tannergooding deleted the backport/pr-104388-to-release/8.0-staging branch July 10, 2024 17:35
@github-actions github-actions bot locked and limited conversation to collaborators Aug 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants