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.

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

Add example: "Filter csv records matching a predicate". #265

Merged
merged 1 commit into from Aug 22, 2017

Conversation

Projects
None yet
2 participants
@bsotodo
Copy link

bsotodo commented Aug 8, 2017

Resolves #230

@bsotodo

This comment has been minimized.

Copy link
Author

bsotodo commented Aug 8, 2017

test encoding_sect_filter_csv_records_matching_a_predicate_line_603 ... FAILED

Travis is complaining about line 603: # quick_main!(run);. Any ideas?

@budziq

This comment has been minimized.

Copy link
Collaborator

budziq commented Aug 9, 2017

Hi. Travis is not complaining about the specific line.
"encoding_sect_filter_csv_records_matching_a_predicate_line_603" is the autogenerated test name (new feature of skeptic test runner). And travis is just reporting that this test fails. Are the tests passing locally on your setup with cargo test? Likely one of the ? int he code returned error.

@budziq

This comment has been minimized.

Copy link
Collaborator

budziq commented Aug 9, 2017

@sotocodes the bail! results in returning from run early with an Err result. quick_main will unwrap the result and there is your panic. While the setup for cli app is sane it is non ideal for a testcase.
We can either make the example no_run or change the example with some hardcoded query.

@bsotodo

This comment has been minimized.

Copy link
Author

bsotodo commented Aug 11, 2017

@budziq cargo test outputs

error: failed to run custom build command for `openssl-sys v0.9.15`
process didn't exit successfully: `/home/username/_Code/OSS/rust-cookbook/target/debug/build/openssl-sys-169fec58669c2940/build-script-build` (exit code: 101)
--- stdout
cargo:rerun-if-env-changed=OPENSSL_LIB_DIR
cargo:rerun-if-env-changed=OPENSSL_INCLUDE_DIR
cargo:rerun-if-env-changed=OPENSSL_DIR
run pkg_config fail: "`\"pkg-config\" \"--libs\" \"--cflags\" \"openssl\"` did not exit successfully: exit code: 1\n--- stderr\nPackage openssl was not found in the pkg-config search path.\nPerhaps you should add the directory containing `openssl.pc\'\nto the PKG_CONFIG_PATH environment variable\nNo package \'openssl\' found\n"

--- stderr
thread 'main' panicked at '

Could not find directory of OpenSSL installation, and this `-sys` crate cannot
proceed without this knowledge. If OpenSSL is installed and this crate had
trouble finding it,  you can set the `OPENSSL_DIR` environment variable for the
compilation process.

If you're in a situation where you think the directory *should* be found
automatically, please open a bug at https://github.com/sfackler/rust-openssl
and include information about your system as well as this message.

    $HOST = x86_64-unknown-linux-gnu
    $TARGET = x86_64-unknown-linux-gnu
    openssl-sys = 0.9.15

', /home/username/.cargo/registry/src/github.com-1ecc6299db9ec823/openssl-sys-0.9.15/build.rs:198
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Build failed, waiting for other jobs to finish...
error: build failed

What do I need OpenSSL for?

@budziq

This comment has been minimized.

Copy link
Collaborator

budziq commented Aug 11, 2017

openssl-sys (ffi wrapper over system OpenSSL) is a build predicate for hyper and reqwest that are used in some other examples. Basically you need to install OpenSSL developement package as described in
https://github.com/sfackler/rust-openssl/blob/master/README.md#building

@bsotodo

This comment has been minimized.

Copy link
Author

bsotodo commented Aug 11, 2017

Alright now I can run the tests, thanks @budziq. I changed the example to use a hardcoded query. It works well but the test still fails:

test encoding_sect_filter_csv_records_matching_a_predicate_line_598 ... FAILED

fn run() -> Result<()> {
    let query = "CA";
    let data = "\
City,State,Population,Latitude,Longitude
Kenai,AK,7610,60.5544444,-151.2583333
Oakman,AL,,33.7133333,-87.3886111
Sandfort,AL,,32.3380556,-85.2233333
West Hollywood,CA,37031,34.0900000,-118.3608333";

    let mut rdr = csv::ReaderBuilder::new().from_reader(data.as_bytes());
    let mut wtr = csv::Writer::from_writer(io::stdout());
    wtr.write_record(rdr.headers()?)?;

    for result in rdr.records() {
        let record = result?;
        if record.iter().any(|field| field == jjjjkkkjjjquery) {
            wtr.write_record(&record)?;
        }
    }

    wtr.flush()?;
    Ok(())
}
@budziq

This comment has been minimized.

Copy link
Collaborator

budziq commented Aug 11, 2017

Hi I see two problems:

  1. you have not pushed the new version so the travis test result is not updated
  2. the code you have pasted has an error (I assume that it was introduced while pasting)
error[E0425]: cannot find value `jjjjkkkjjjquery` in this scope
  --> src/main.rs:30:47
   |
30 |         if record.iter().any(|field| field == jjjjkkkjjjquery) {
   |                                               ^^^^^^^^^^^^^^^ not found in this scope

Please update the PR with newest code and if the problems persist I'll look at the problem :)

@bsotodo bsotodo force-pushed the bsotodo:ex-csv-filter branch from 683c8de to f03dab7 Aug 11, 2017

@budziq
Copy link
Collaborator

budziq left a comment

@sotocodes sorry for the delay. I do not get notifications when new commit arrives. Only after comments in the PR thread. So please ping me next time :)

The code looks great. but there are some minor suggestions to the description:


_Disclaimer: this example is modified version of [this](https://docs.rs/csv/*/csv/tutorial/index.html#filter-by-search) one._

Takes a `query` from the command line as input and outputs only the

This comment has been minimized.

@budziq

budziq Aug 16, 2017

Collaborator

Takes a query from the command line as input and outputs only the
well its not true anymore ;)

This comment has been minimized.

@bsotodo

bsotodo Aug 22, 2017

Author

ops 🤦‍


[![csv-badge]][csv] [![cat-encoding-badge]][cat-encoding]

_Disclaimer: this example is modified version of [this](https://docs.rs/csv/*/csv/tutorial/index.html#filter-by-search) one._

This comment has been minimized.

@budziq

budziq Aug 16, 2017

Collaborator

I would move the disclaimer to bottom of the text or example and reword the text to:
"Disclaimer: this example has been adapted from the csv crate tutorial ."

@bsotodo bsotodo force-pushed the bsotodo:ex-csv-filter branch from f03dab7 to bdd2697 Aug 22, 2017

@bsotodo

This comment has been minimized.

Copy link
Author

bsotodo commented Aug 22, 2017

Hey @budziq, I made the changes but it seems that AppVeyor does not like them :)

ping me next time :)

roger that!

@bsotodo bsotodo closed this Aug 22, 2017

@bsotodo bsotodo reopened this Aug 22, 2017

@budziq

This comment has been minimized.

Copy link
Collaborator

budziq commented Aug 22, 2017

nicely done!

@budziq budziq merged commit 588e7cb into rust-lang-nursery:master Aug 22, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bsotodo

This comment has been minimized.

Copy link
Author

bsotodo commented Aug 22, 2017

thanks for the help :)

@budziq

This comment has been minimized.

Copy link
Collaborator

budziq commented Aug 22, 2017

no problem. keep 'em coming :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.