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

Levenshtein-distance: New exercise suggestion #1634

Closed
wants to merge 2 commits into from
Closed

Levenshtein-distance: New exercise suggestion #1634

wants to merge 2 commits into from

Conversation

marcelritzschke
Copy link

No description provided.

@rpottsoh rpottsoh added the hold label Feb 2, 2020
Copy link
Member

@rpottsoh rpottsoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick glance, I see some minor formatting changes that would be good for the canonical data.

exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
exercises/levenshtein-distance/canonical-data.json Outdated Show resolved Hide resolved
@rpottsoh
Copy link
Member

rpottsoh commented Feb 2, 2020

It would be a nice if the opening comment to this PR gave some background information regarding this exercise.

Also, presently, adding new exercises is on hold in a general sense for the site. However, it doesn't mean that individual language tracks can't still add this exercise now if they so choose.

@rpottsoh rpottsoh changed the title Adding exercise Levenshtein distance Levenshtein-distance: New exercise suggestion Feb 2, 2020
@marcelritzschke
Copy link
Author

Thanks a lot for reviewing of my PR! I tried to follow your suggestions

@yawpitch
Copy link
Contributor

yawpitch commented Feb 2, 2020

We can't merge this until #1560 lifts, but it's a great idea for an exercise. My one thought, should it just be levenshtein rather than levenshtein-distance, along the same lines as hamming not being hamming-distance?

@marcelritzschke
Copy link
Author

@yawpitch Renaming du just levenshtein is a good point, since I don't know anything else which is associated to Levenshtein. I will do the changes and we can wait for #1560

@petertseng
Copy link
Member

As of f18d301, the cases in this file are correct, according to:

require 'json'
require_relative '../../verify'

# https://web.archive.org/web/20180612143641/https://bitbucket.org/clearer/iosifovich/
def lev_dist(shorter, longer)
  shorter, longer = [longer, shorter] if shorter.size > longer.size
  left = 0
  right = -1
  left += 1 while shorter[left] &.== longer[left]
  right -= 1 while left - right <= shorter.size && shorter[right] == longer[right]

  buf = Array.new(shorter.size + 2 - left + right, 0)
  #puts "compare #{left} to #{right}: #{shorter[left..right]} vs #{longer[left..right]} (#{buf.size})"

  (left..(longer.size + right)).each { |i|
    clong = longer[i]
    tmp = buf[0]
    buf[0] += 1

    (1...buf.size).each { |j|
      cshort = shorter[left + j - 1]
      r = buf[j]
      buf[j] = [
        r + 1,
        buf[j - 1] + 1,
        tmp + (clong == cshort ? 0 : 1),
      ].min
      tmp = r
    }
  }

  buf[-1]
end

json = JSON.parse(File.read(File.join(__dir__, 'canonical-data.json')))

verify(json['cases'], property: 'distance') { |i, _|
  lev_dist(i['from'], i['to'])
}

@SleeplessByte
Copy link
Member

@n4bla #1560 has been resolved!

@SleeplessByte
Copy link
Member

Closed and re-opened to make CI run.

Base automatically changed from master to main January 27, 2021 15:31
@ErikSchierboom
Copy link
Member

@marcelritzschke Are you still interested in working on this PR? If so, I've created a branch that adds four new commits that get your PR up-to-date with the latest specs: https://github.com/exercism/problem-specifications/commits/exercise-levenshtein-distance You chould cherry-pick those commits onto your branch to update your PR.

@IsaacG
Copy link
Member

IsaacG commented Dec 30, 2022

This appears abandoned. I'll close this out. If you still want to work on this, we can reopen.

@IsaacG IsaacG closed this Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants