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

Use generic math in System.Numerics.Vectors test utils #60365

Closed
Tracked by #63548
akoeplinger opened this issue Oct 13, 2021 · 5 comments · Fixed by #60905
Closed
Tracked by #63548

Use generic math in System.Numerics.Vectors test utils #60365

akoeplinger opened this issue Oct 13, 2021 · 5 comments · Fixed by #60905
Labels
area-System.Numerics help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@akoeplinger
Copy link
Member

See #60335 (comment), it could simplify the code a lot.

@akoeplinger akoeplinger added area-System.Numerics help wanted [up-for-grabs] Good issue for external contributors labels Oct 13, 2021
@akoeplinger akoeplinger added this to the Future milestone Oct 13, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Oct 13, 2021
@ghost
Copy link

ghost commented Oct 13, 2021

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

Issue Details

See #60335 (comment), it could simplify the code a lot.

Author: akoeplinger
Assignees: -
Labels:

area-System.Numerics, up-for-grabs, untriaged

Milestone: Future

@akoeplinger akoeplinger removed the untriaged New issue has not been triaged by the area owner label Oct 13, 2021
@SkiFoD
Copy link
Contributor

SkiFoD commented Oct 26, 2021

@akoeplinger Hi, I would like to work on the issue and I have already done some research. I have trouble with adding the System.Runtime.Experimental reference to the project so I could use the generic math. Could you give me a hint how to do that the right way?

@tannergooding
Copy link
Member

@SkiFoD, you should just need to add a reference to the System.Runtime.Experimental reference assembly, like so

    <!-- it's a reference assembly, but the project system doesn't know that - include it during compilation, but don't publish it -->
    <ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.Experimental\ref\System.Runtime.Experimental.csproj" IncludeAssets="compile" Private="false" />

This is how its handle in the corresponding test project for System.Runtime.Experimental: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime.Experimental/tests/System.Runtime.Experimental.Tests.csproj#L51-L52

@SkiFoD
Copy link
Contributor

SkiFoD commented Oct 26, 2021

@tannergooding Thank you! I have some more questions. Is it ok to wrap the Util.cs with pragmas that disables CA2252 or is there a better way to get rid of the warnings?

@tannergooding
Copy link
Member

You'll need to annotate the type with RequiresPreviewFeatures attribute, much as was already done for https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime/tests/System/GenericMathHelpers.cs

These will go away once we start stabilizing the feature and confirm it will no longer be preview in the corresponding .NET release.

SkiFoD added a commit to SkiFoD/runtime that referenced this issue Oct 27, 2021
@tannergooding tannergooding linked a pull request Oct 27, 2021 that will close this issue
SkiFoD added a commit to SkiFoD/runtime that referenced this issue Oct 27, 2021
SkiFoD added a commit to SkiFoD/runtime that referenced this issue Oct 28, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 3, 2021
tannergooding added a commit that referenced this issue Feb 11, 2022
* Use generic math in System.Numerics.Vectors (#60365)

* Add more use of the Generic Math (#60365)

* Got rid of the dynamic cast (#60365)

Co-authored-by: Tanner Gooding <tagoo@outlook.com>
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 11, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants