Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

mat64: reverse signatures for decomposition matrix extractions #436

Closed
wants to merge 2 commits into from

Conversation

kortschak
Copy link
Member

Also provides some sugar to ease this.

@btracey @vladimir-ch Please take a look.

Closes #435.

@btracey
Copy link
Member

btracey commented Mar 19, 2017

Why the "To" endings? It seems like L (for instance) is just as clear.

@kortschak
Copy link
Member Author

There are two usage syntaxes, dst = x.PTo(nil) and x.PTo(dst). The first is obviously an assignment to dst, but the second would be less obvious in the case that it were written x.P(y). The To suffix clears that use intention (at a minor cost in the first case where To(nil) is a little upsetting, but I think the trade-off is worth it (and could be apologised). It lends its origin from io.WriterTo.

@kortschak
Copy link
Member Author

I'll wait for @btracey to make sure he's happy with my argument above.

@btracey
Copy link
Member

btracey commented Mar 25, 2017

Thinking about it, I think the related argument is that usually the receiver is modified. This is a difference, so that difference should be clear.

@btracey
Copy link
Member

btracey commented Mar 25, 2017

LGTM

@btracey
Copy link
Member

btracey commented Mar 25, 2017

This is a pretty big breaking change, at least for code I've written. Is it possible we could merge repos into gonum/gonum and then make this change? It sounds like there's nothing in the way of migration that will change.

@btracey btracey mentioned this pull request Mar 25, 2017
@kortschak
Copy link
Member Author

I'm happy to wait. This is relatively small and I'll just copy past over to the new location when we are ready if that's what you want.

@kortschak
Copy link
Member Author

Curious though. How does delaying the merge of this help?

@btracey
Copy link
Member

btracey commented Mar 27, 2017

My suggestion was that this is a (decent-sized) breaking change, and it might be nice to delay that until the merge. This way, old code can remain working without update.

@kortschak
Copy link
Member Author

Done in gonum/gonum.

@kortschak kortschak closed this Jun 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants