Skip to content

Allow boolean parsing of strings containing 0 and 1#29997

Merged
JeffBezanson merged 1 commit intoJuliaLang:masterfrom
kim366:master
Nov 28, 2018
Merged

Allow boolean parsing of strings containing 0 and 1#29997
JeffBezanson merged 1 commit intoJuliaLang:masterfrom
kim366:master

Conversation

@kim366
Copy link
Copy Markdown
Contributor

@kim366 kim366 commented Nov 11, 2018 •
edited
Loading

Fixes #29980

The following tests pass:

@test parse(Bool, "1") == true
@test parse(Bool, "01") == true
@test parse(Bool, "0") == false
@test parse(Bool, "000000000000000000000000000000000000000000000000001") == true
@test parse(Bool, "000000000000000000000000000000000000000000000000000") == false
@test_throws ArgumentError parse(Bool, "1000000000000000000000000000000000000000000000000000")
@test_throws ArgumentError parse(Bool, "2")
@test_throws ArgumentError parse(Bool, "02")

@ararslan ararslan added needs tests Unit tests are required for this change needs news A NEWS entry is required for this change minor change Marginal behavior change acceptable for a minor release labels Nov 11, 2018
@kim366
Copy link
Copy Markdown
Contributor Author

kim366 commented Nov 12, 2018

@ararslan Tests added

ararslan
ararslan reviewed Nov 12, 2018
View reviewed changes
@ararslan ararslan removed the needs tests Unit tests are required for this change label Nov 12, 2018
@StefanKarpinski
Copy link
Copy Markdown
Member

StefanKarpinski commented Nov 13, 2018

This seems to break the parsing of JULIA_CPU_THREADS quite badly.

StefanKarpinski
StefanKarpinski previously requested changes Nov 13, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@StefanKarpinski StefanKarpinski left a comment

Choose a reason for hiding this comment

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

Please investigate the JULIA_CPU_THREADS parsing failure.

@kim366
Copy link
Copy Markdown
Contributor Author

kim366 commented Nov 13, 2018

@StefanKarpinski hm, can you point me to some specific file or give me an example?

@ararslan
Copy link
Copy Markdown
Member

ararslan commented Nov 13, 2018

Take a look at the CI job logs to see the failures.

Though it appears other PRs are experiencing this as well, so I'm inclined to think something else has introduced this problem on master.

@StefanKarpinski
Copy link
Copy Markdown
Member

StefanKarpinski commented Nov 13, 2018

https://build.julialang.org/#/builders/68/builds/794/steps/8/logs/stdio

@kim366
Copy link
Copy Markdown
Contributor Author

kim366 commented Nov 13, 2018

Yeah, I'm getting very similar CI output to the newest master commit and all parse tests are passing

@iblislin
Copy link
Copy Markdown
Member

iblislin commented Nov 14, 2018

FreeBSD CI passed now. Sorry for that parsing failure.

@StefanKarpinski
Copy link
Copy Markdown
Member

StefanKarpinski commented Nov 14, 2018

Failures are #29923.

@ararslan ararslan dismissed StefanKarpinski’s stale review November 14, 2018 21:40

Requested changes were not related to this PR.

@JeffBezanson JeffBezanson added this to the 1.1 milestone Nov 28, 2018
@JeffBezanson
Copy link
Copy Markdown
Member

JeffBezanson commented Nov 28, 2018

Thanks, @kim366 ! I'll add news for this.

@JeffBezanson JeffBezanson merged commit d4c6d16 into JuliaLang:master Nov 28, 2018
@kim366
Copy link
Copy Markdown
Contributor Author

kim366 commented Nov 28, 2018

My first merged pull request <3 one life goal reached

@ararslan
Copy link
Copy Markdown
Member

ararslan commented Nov 28, 2018

Good work here and thanks for the contribution! I hope you'll continue to contribute to Julia. 🙂

@JeffBezanson JeffBezanson removed the needs news A NEWS entry is required for this change label Nov 29, 2018
fredrikekre added a commit that referenced this pull request Nov 30, 2018
@fredrikekre
News and compat annotation for #29997 (parse(Bool, str)).
799f712
fredrikekre added a commit that referenced this pull request Dec 1, 2018
@fredrikekre
News and compat annotation for #29997 (parse(Bool, str)).
219f277
fredrikekre added a commit that referenced this pull request Dec 1, 2018
@fredrikekre
News and compat annotation for #29997 (parse(Bool, str)).
97a1edf
fredrikekre added a commit that referenced this pull request Dec 3, 2018
@fredrikekre
News and compat annotation for #29997 (parse(Bool, str)).
7f4add8
fredrikekre added a commit that referenced this pull request Dec 4, 2018
@fredrikekre
News and compat annotation for #29997 (parse(Bool, str)).
8cf4b34
fredrikekre added a commit that referenced this pull request Dec 4, 2018
@fredrikekre
News and compat annotation for #29997 (parse(Bool, str)).
09a6710
fredrikekre added a commit that referenced this pull request Dec 5, 2018
@mortenpi @fredrikekre
Addition of NEWS and compat admonitions for important
cceb675 
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
fredrikekre added a commit that referenced this pull request Dec 5, 2018
@mortenpi @fredrikekre
Addition of NEWS and compat admonitions for important
ebbabf5 
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
fredrikekre added a commit that referenced this pull request Dec 5, 2018
@fredrikekre @mortenpi
Compat admonitions and NEWS for Julia 1.1 (#30230)
6175bd9 
Addition of NEWS and compat admonitions for important changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.


Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ararslan ararslan ararslan left review comments

@StefanKarpinski StefanKarpinski StefanKarpinski left review comments

Assignees

No one assigned

Labels

minor change Marginal behavior change acceptable for a minor release

Projects

None yet

Milestone

1.1

Development

Successfully merging this pull request may close these issues.

5 participants

@kim366 @StefanKarpinski @ararslan @iblislin @JeffBezanson