fossasia / knittingpattern

A Python Library for Knitting Patterns
https://pypi.python.org/pypi/knittingpattern
GNU Lesser General Public License v3.0
1.57k stars 30 forks source link

measure and change unique #66

Open niccokunzmann opened 8 years ago

niccokunzmann commented 8 years ago

Do a performance measurement of both implementations:

depending on what is faster in CPython3.5, use it as standard implementation.

jainaman224 commented 7 years ago

Can we use python set...? https://docs.python.org/3/library/stdtypes.html#set

niccokunzmann commented 7 years ago

sure.

niccokunzmann commented 7 years ago

In #73 is a discussion relating to this.

niccokunzmann commented 7 years ago

@jainaman224 You can use sets. I do not care so much how it is implemented this issue is about making it faster.

jainaman224 commented 7 years ago
@pytest.mark.parametrize("input,expected_result", [
        ([], []), ([[1, 1, 1, 1, 1]], [1]),
        ([[1, 2, 3], [4, 3, 2, 1]], [1, 2, 3, 4]),
        ([[None, 4], [4, 6, None]], [None, 4, 6]),
        ([[[], [1]], [[1], [0], []]], [[], [1], [0]])])

I can't use set or map because we are using different nesting for result and including None as unique element.

It should be according to me:

@pytest.mark.parametrize("input,expected_result", [
        ([], []), ([[1, 1, 1, 1, 1]], [1]),
        ([[1, 2, 3], [4, 3, 2, 1]], [1, 2, 3, 4]),
        ([[None, 4], [4, 6, None]], [4, 6]),
        ([[[], [1]], [[1], [0], []]], Error)])

@niccokunzmann What do you think...?

niccokunzmann commented 7 years ago
([[[], [1]], [[1], [0], []]], Error)])

The last case is in case people specify a color as a list. However, this may not be of interest for now. If you think, we should remove the test or test an error, that is fine. I do not know if it is useful.

([[None, 4], [4, 6, None]], [None, 4, 6]),

The question is: is no color also a color? My answer is yes: some patterns may not specify a color, so None is also a color. What do you think? We could argue that "for all instructions in a row: the color of the instruction must be present in the row.instruction_colors"

jainaman224 commented 7 years ago

I also agree upon this. No color is also a color. So, we can assign a special integer for it. As None is not accepted by set as element. Is it ok...?

niccokunzmann commented 7 years ago
>>> s  = set()
>>> s.add(None)
>>> 

None is accepted.

jainaman224 commented 7 years ago

Okay, There was error due to sorting. Sorry for misunderstanding.