Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Require distances keyword for MDS #131

Merged
merged 2 commits into from
Nov 15, 2020
Merged

Require distances keyword for MDS #131

merged 2 commits into from
Nov 15, 2020

Conversation

timholy
Copy link
Collaborator

@timholy timholy commented Oct 27, 2020

fit(MDS, ...) lacked documentation, and consequently I simply assumed
that one passed a matrix of distances. This did not trigger any errors, but the
results were nonsense, because the distances matrix was interpreted as
a data matrix.

We can address this by requiring the user to provide the distances keyword.

Note that this is a breaking change and so I've taken liberty to bump the minor version.

@timholy timholy changed the title Update [compat] and add CompatHelper Require distances keyword for MDS Oct 27, 2020
`fit(MDS, ...)` lacked documentation, and consequently I simply assumed
that one  passed a matrix of distances. This did not trigger any errors, but the
results were nonsense. We can address this by requiring the user to
provide the `distances` keyword.
@timholy
Copy link
Collaborator Author

timholy commented Oct 27, 2020

Test failure seems unrelated.

@timholy
Copy link
Collaborator Author

timholy commented Nov 12, 2020

Friendly bump. Anything I need to do to get this merged?

@ararslan
Copy link
Member

This package is rather undermaintained. I've invited you for write access. The changes here look reasonable to me but I'm not familiar with this method.

@timholy
Copy link
Collaborator Author

timholy commented Nov 12, 2020

Thanks! I'll give it another couple of days and then merge if no one complains.

@ararslan
Copy link
Member

Eh, people have had 16 days to complain; if they were going to, they probably already would have 😄

@timholy
Copy link
Collaborator Author

timholy commented Nov 12, 2020

Yeah, but now they know I'm dangerous 😈

@timholy timholy merged commit 9ae28d0 into JuliaStats:master Nov 15, 2020
@timholy timholy deleted the teh/mdsreconstruct branch November 15, 2020 18:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants