Drop the implicit pair to Interval conversion - #869
Merged
Conversation
Closes #861. #853 removed the conversion from List<Entity> because it made three params overloads uncallable, and kept the one from Entity[] on the narrow ground that it never produced that ambiguity -- an array binds to the params overload in its normal form by an identity conversion, which wins outright. True, and beside the point, as Happypig375 pointed out on the issue: an array carries an order and can repeat an element, a set has neither, so the conversion silently discarded part of what it was handed. That disqualifies it on its own. The question to ask of an implicit conversion is what it loses, not whether it breaks an overload today, and set types are built explicitly nearly everywhere for this reason. Measured before writing anything: removing it broke exactly one place in the repository, which was the test pinning it. Nothing in the library, no other test, no sample. That test now says the opposite, and still earns its keep the same way -- the file compiling is what says the conversion has not come back. Verified: 6084 C# tests and 130 F# tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #867. A two-element tuple became an Interval with both endpoints included. A tuple says nothing about whether either endpoint is in, so the conversion had to supply that, and it chose the reading opposite to the notation: (1, 5) is the open interval in ordinary mathematical writing, and this produced the closed one. A caller writing what looks like an open interval got a closed one, with 1 a member of it, and nothing said so. Where the array conversion removed in #866 dropped information that was there, this one produced information that was not. Same fault from the other side, and the same remedy -- ask for the interval by name, which makes the choice visible instead of assumed: MathS.Interval(1, 5) closed MathS.Interval(1, false, 5, false) open Measured before writing anything: nothing in the library, the tests, the F# wrapper or the utilities used it. Zero compile errors on removal, where #866 had one. The other pairs in the API are left alone, and checked rather than assumed: an integration Range, the arguments of Substitute, the cases of MathS.Piecewise and ToProvided are all ordered pairs whose halves have distinct stated roles. None of them has to guess what it was handed. Verified: 6085 C# tests and 130 F# tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…terval-conversion # Conflicts: # BREAKING-CHANGES.md # Sources/AngouriMath/Core/Entity/Omni/Entity.Set.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #867. Depends on #866 — branched from it, since both remove a line from the same
#pragmablock inEntity.Set.cs.A two-element tuple became an
Intervalwith both endpoints included:A tuple says nothing about whether either endpoint is in, so the conversion had to supply that — and it chose the reading opposite to the notation.
(1, 5)is the open interval in ordinary mathematical writing and[1, 5]the closed one, so a caller writing what looks like an open interval got a closed one, silently.Where the array conversion removed in #866 dropped information that was there, this one produced information that was not. Same fault from the other side, and the same remedy — ask for the interval by name, which makes the choice visible instead of assumed:
Blast radius, measured before writing anything
Zero compile errors on removal — nothing in the library, the tests, the F# wrapper or
Utilsused it. (#866 had one.)The other pairs are left alone, and checked rather than assumed
Nine other public members take an
(Entity, Entity). None of them has to guess what it was handed — each half has a distinct, stated role:Integralf.RangeSubstituteMathS.PiecewiseToProvidedExpandSineOfSum/ExpandCosineOfSumOnly the interval conversion had an ambiguity to resolve, which is why only it goes.
Verification
PublicApiSurfaceTestgreen,PublicApi.txtupdatedThe new assertion in
ImplicitOperatorsis secondary; the load-bearing part is that the file compiles, which is what fails if the conversion comes back.Breaking, so it wants the 2.0 window while it is still in preview.
🤖 Generated with Claude Code