[f] Add expect(str).to.similar_to(str) - Githubissues

EastAgile / robber.py

BDD / TDD assertion library for Python

MIT License

8 stars 1 forks source link

[f] Add expect(str).to.similar_to(str) #41

Closed catriuspham closed 7 years ago

catriuspham commented 7 years ago

Related issue

https://github.com/EastAgile/robber.py/issues/37

Description

Let's add this expectation to our code base:

expect(str).to.be.similar_to(str)

This asserts if two strings are similar. similar means after removing all the spaces and got lowercased, the two strings are exactly the same

Usage

expect('  a  CA t   ').to.be.similar_to('a c A  t')

seri-ea commented 7 years ago

Can we call it be_similar_to? to similar to sounds awkward.

coveralls commented 7 years ago

Coverage decreased (-0.4%) to 99.598% when pulling e87a9ae80d6d6286bfe56ec91df338155659bc97 on feature/string-similar into 849000fe56a960a02e8382b9cdb7479a3abe06c4 on feature/explaination-system.

catriuspham commented 7 years ago

@seri-ea You can use to.be.similar_to. Nice catch, though.

coveralls commented 7 years ago

Coverage decreased (-0.4%) to 99.598% when pulling dc4db3acda579fb55c0a7ee2d24c03704e28b82c on feature/string-similar into 849000fe56a960a02e8382b9cdb7479a3abe06c4 on feature/explaination-system.

seri-ea commented 7 years ago

I see @catriuspham Never mind. I think we can leave it this way. Maybe we should use the better style (be.similar_to) in documentation though.

coveralls commented 7 years ago

Coverage remained the same at 100.0% when pulling 47a47d35a96d0cccc84213c9ba3a4d7be0e57f68 on feature/string-similar into 849000fe56a960a02e8382b9cdb7479a3abe06c4 on feature/explaination-system.

coveralls commented 7 years ago

Coverage remained the same at 100.0% when pulling 47a47d35a96d0cccc84213c9ba3a4d7be0e57f68 on feature/string-similar into 849000fe56a960a02e8382b9cdb7479a3abe06c4 on feature/explaination-system.

coveralls commented 7 years ago

Coverage remained the same at 100.0% when pulling 47a47d35a96d0cccc84213c9ba3a4d7be0e57f68 on feature/string-similar into 849000fe56a960a02e8382b9cdb7479a3abe06c4 on feature/explaination-system.

coveralls commented 7 years ago

Coverage remained the same at 100.0% when pulling 43720b049a7f12d51a231452f2db18120e24a6e1 on feature/string-similar into 849000fe56a960a02e8382b9cdb7479a3abe06c4 on feature/explaination-system.

coveralls commented 7 years ago

Coverage remained the same at 100.0% when pulling 43720b049a7f12d51a231452f2db18120e24a6e1 on feature/string-similar into 849000fe56a960a02e8382b9cdb7479a3abe06c4 on feature/explaination-system.

hieueastagile commented 7 years ago

Hm. I'm really sorry but I think we should not merge this PR.

First of all, I don't think it's pythonic one. Obviously, when I'm trying to compare if two strings are the same after being standardized, I will do something like, it's more natural to me. And I prefer this is the only way.
```
s1 = standardize(...)
s2 = standardize(...)
expect(s1).to.be.eq(s2)
```
Other point, I don't think we should do expect for something similar, it's awkward to me to write something like:
```
expect(something).to.be.similar(the_other)
```
I think it should be
```
expect(similarity(something, the_other)).to.be.less_than(0.8)
```
for example. Because in test, we want to check something precisely, exactly equal, rather than similar.
And the last, it's easy to refer to string similarity when you use be.similar_to(...), that means something like levenshtein or edit or jaro-winkler distance than just standardize string in some way.

Then, please let me know more about your idea about these points.

seri-ea commented 7 years ago

@hieueastagile On the first point: Actually I was about to make the exact same comment. This matcher doesn't seem general enough. I mean, what is the definition on string similarity after all? No one can agree on a standard.

But then you said "String similarity is great, we can implement it." on https://github.com/EastAgile/robber.py/issues/37 so I skipped it. You probably thought of something else on that issue page. (Edit-distance as your last point).

So anyway, agree that this PR as-is should not be merged.

hieueastagile commented 7 years ago

What I refer to similarity is my last idea at that time.

catriuspham commented 7 years ago

Actually, I got the idea straight from Sure (http://sure.readthedocs.io/en/latest/api-reference.html#look-like). I also read through the source code though, what they do is exactly the same as I (remove all the spaces using regex then lowercase the strings). However, I get your point, this is a short PR with little effort, so closing it is no big deal for me :sweat_smile: