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.

Sign up for GitHub

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

Jump to bottom

Allow boolean parsing of strings containing 0 and 1 #29997

Merged
merged 1 commit into from
Nov 28, 2018
Merged

Allow boolean parsing of strings containing 0 and 1 #29997

merged 1 commit into from
Nov 28, 2018

Conversation

kim366
Copy link
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 kind:minor change Marginal behavior change acceptable for a minor release labels Nov 11, 2018
@kim366
Copy link
Contributor Author

kim366 commented Nov 12, 2018

@ararslan Tests added

ararslan
ararslan reviewed Nov 12, 2018
View reviewed changes
test/parse.jl Outdated Show resolved Hide resolved
@ararslan ararslan removed the needs tests Unit tests are required for this change label Nov 12, 2018
@StefanKarpinski
Copy link
Sponsor Member

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
Sponsor 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
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
Member

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
Sponsor Member

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

@kim366
Copy link
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
Member

FreeBSD CI passed now. Sorry for that parsing failure.

@StefanKarpinski
Copy link
Sponsor Member

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
Sponsor Member

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

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

kim366 commented Nov 28, 2018

My first merged pull request <3 one life goal reached

@ararslan
Copy link
Member

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
kind: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