Skip to content

Conversation

@kellrott
Copy link
Contributor

Added split method to str:

>>> "hello|world".split("|") ['hello', 'world'] 

@codecov-io
Copy link

codecov-io commented May 17, 2019

Codecov Report

Merging #60 into master will increase coverage by 0.24%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #60 +/- ## ========================================== + Coverage 67.94% 68.18% +0.24%  ========================================== Files 59 59 Lines 10378 10423 +45 ========================================== + Hits 7051 7107 +56 + Misses 2828 2813 -15 - Partials 499 503 +4
Impacted FilesCoverage Δ
py/string.go88.2% <100%> (+2.88%)⬆️
py/type.go51.54% <0%> (+0.32%)⬆️
py/sequence.go36.55% <0%> (+2.15%)⬆️
py/list.go25.44% <0%> (+4.14%)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61059b4...e316b2b. Read the comment docs.

@corona10corona10 requested a review from ncwMay 18, 2019 01:43
Copy link
Collaborator

@ncwncw left a comment

Choose a reason for hiding this comment

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

I think that is an excellent start :-)

However there is quite a bit of str.split functionality missing which I've outlined - fancy adding it?

py/string.go Outdated
}
return&o, nil
}
returnnil, fmt.Errorf("Not split by string")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be a TypeError

>>> str.split(8) Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: descriptor 'split' requires a 'str' object but received a 'int' 

return&o, nil
}
returnnil, fmt.Errorf("Not split by string")
}, 0, "split(sub) -> split string with sub.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Python help is

 S.split([sep [,maxsplit]]) -> list of strings Return a list of the words in the string S, using sep as the delimiter string. If maxsplit is given, at most maxsplit splits are done. If sep is not specified or is None, any whitespace string is a separator and empty strings are removed from the result. 

Which makes me see that we are missing two things.

  • If sep isn't passed (or is None) in then we should be using strings.Fields in go terms (This doesn't have a maxfields parameter though.)
  • There is another optional parameter to specify the number of splits.

Here are some cases to consider

>>> "a,d,c".split(",") ['a', 'd', 'c'] >>> "a,d,c".split(",",1) ['a', 'd,c'] >>> " a d b ".split() ['a', 'd', 'b'] >>> " a d b ".split(None, 1) ['a', 'd b '] 

ncw
ncw approved these changes May 28, 2019
@ncw
Copy link
Collaborator

ncw commented May 28, 2019

That looks great now thank you and 100% diff coverage too :-)

I'll merge it now - thank you very much.

@ncwncw merged commit f76e37b into go-python:masterMay 28, 2019
@kellrottkellrott deleted the string-split branch December 24, 2022 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kellrott@codecov-io@ncw