-
Notifications
You must be signed in to change notification settings - Fork 28
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
update geo dependencies #743
Conversation
src/utils.rs
Outdated
wkt.items | ||
.pop() | ||
.unwrap() | ||
.try_into() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now utilize idiomatic Rust's TryInto
trait, rather than having our own function only for wkt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to replace it with this
let wkt = wkt::Wkt::from_str(&s).map_err(serde::de::Error::custom)?;
use std::convert::TryInto;
wkt.try_into().map_err(serde::de::Error::custom)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed! Fixed.
Do you think the WKT crate should add methods like these for optional serde
integration? It seems likely to be useful to other people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually might be helpful indeed, so I took the opportunity to contribute back. See georust/wkt#59.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful! I'll take a look.
I should add, three tests failed locally for me, but I did not investigate, since the same ones failed in the same way before these changes.
full outputrunning 205 tests test add_prefix::tests::collection_referential ... ok test add_prefix::tests::collection_referential_and_schedule ... ok test add_prefix::tests::collection_deprecated ... ok test add_prefix::tests::collection_no_prefix ... ok test add_prefix::tests::collection_schedule ... ok test add_prefix::tests::collection_with_id_no_prefix ... ok test add_prefix::tests::collection_with_id_referential ... ok test add_prefix::tests::collection_with_id_schedule ... ok test add_prefix::tests::collection_with_id_referential_and_schedule ... ok test add_prefix::tests::collection_with_id_deprecated ... ok test enhancers::fill_co2::test::preserve_existing ... ok test enhancers::fill_co2::test::enhance_with_default ... ok test enhancers::fill_co2::test::add_fallback_modes ... ok test gtfs::read::tests::filter_pathway ... ok test gtfs::read::tests::gtfs_routes_with_no_trips ... ok test gtfs::read::tests::deduplicate_funicular_physical_mode ... ok test gtfs::read::tests::gtfs_routes_as_route_same_name_different_agency ... ok test gtfs::read::tests::gtfs_routes_as_route_with_backward_trips ... ok test gtfs::read::tests::gtfs_routes_as_line ... ok test gtfs::read::tests::gtfs_routes_as_route ... ok test gtfs::read::tests::gtfs_invalid_undefined_stop_times ... ok test gtfs::read::tests::gtfs_routes_without_agency_id_as_line_and_0_agencies ... ok test gtfs::read::tests::gtfs_routes_with_wrong_colors ... ok test gtfs::read::tests::gtfs_routes_without_agency_id_as_line_and_2_agencies ... ok test gtfs::read::tests::gtfs_routes_without_agency_id_as_line ... ok test gtfs::read::tests::gtfs_trips ... ok test gtfs::read::tests::gtfs_stop_times ... ok test gtfs::read::tests::gtfs_stop_times_estimated ... ok test gtfs::read::tests::gtfs_stop_times_precision ... ok test gtfs::read::tests::gtfs_trips_with_no_accessibility_information ... ok test gtfs::read::tests::gtfs_trips_no_direction_id ... ok test gtfs::read::tests::gtfs_without_calendar_dates_or_calendar ... ok test gtfs::read::tests::gtfs_with_calendars_dates_and_no_calendar ... ok test gtfs::read::tests::gtfs_with_calendars_and_no_calendar_dates ... ok test gtfs::read::tests::gtfs_with_calendars_and_calendar_dates ... ok test gtfs::read::tests::gtfs_trips_with_routes_without_agency_id ... ok test gtfs::read::tests::load_2_agencies_with_different_timezone ... ok test gtfs::read::tests::load_2_agencies_with_no_id ... ok test gtfs::read::tests::load_minimal_agency ... ok test gtfs::read::tests::load_complete_agency ... ok test gtfs::read::tests::load_one_stop_point ... ok test gtfs::read::tests::push_on_collection ... ok test gtfs::read::tests::load_standard_agency ... ok test gtfs::read::tests::location_type_default_value ... ok test gtfs::read::tests::gtfs_undefined_stop_times ... ok test gtfs::read::tests::location_with_space_proof ... ok test gtfs::read::tests::no_stop_code_on_autogenerated_stoparea ... ok test gtfs::read::tests::load_without_slashes ... ok test gtfs::read::tests::read_levels ... ok test gtfs::read::tests::read_shapes_with_no_shapes_file ... ok test gtfs::read::tests::read_shapes ... ok test gtfs::read::tests::stop_location_on_stops ... ok test gtfs::read::tests::read_tranfers ... ok test gtfs::write::tests::ntfs_geometry_not_linestring_not_exported ... ok test gtfs::write::tests::ntfs_geometry_linestring_exported ... ok test gtfs::read::tests::stop_code_on_stops ... ok test gtfs::read::tests::stops_do_not_generate_duplicate_equipments ... ok test gtfs::write::tests::ntfs_line_with_unknown_mode_to_gtfs_route ... ok test gtfs::write::tests::ntfs_minial_line_to_gtfs_route ... ok test gtfs::read::tests::read_gtfs_routes::read_gtfs_routes_as_line ... ok test gtfs::write::tests::ntfs_physical_mode_to_gtfs_route_type ... ok test gtfs::read::tests::read_gtfs_routes::read_gtfs_routes_as_route ... ok test gtfs::write::tests::ntfs_transfers_to_gtfs_transfers ... ok test gtfs::write::tests::test_ntfs_minimal_stop_point_to_gtfs_stop ... ok test gtfs::write::tests::ntfs_object_code_to_stop_extensions_nothing_generated ... ok test gtfs::write::tests::test_ntfs_stop_area_to_gtfs_stop ... ok test gtfs::write::tests::test_ntfs_stop_point_to_gtfs_stop ... ok test gtfs::write::tests::write_agency ... ok test gtfs::write::tests::write_agency_with_default_values ... ok test gtfs::write::tests::ntfs_tranfers_at_same_stop_point ... ok test gtfs::write::tests::ntfs_object_code_to_stop_extensions ... ok test gtfs::write::tests::ntfs_vehicle_journeys_to_stop_times ... ok test gtfs::read::tests::stops_generates_equipments ... ok test model::tests::calendar_deduplication::enhance ... ok test model::tests::check_geometries_coherence::preserve_valid_reference ... ok test model::tests::check_geometries_coherence::remove_dead_reference ... ok test model::tests::clean_comments::remove_empty_comment ... ok test gtfs::write::tests::write_calendar_file_from_calendar ... ok test gtfs::write::tests::write_trip ... ok test model::tests::enhance_pickup_dropoff::no_stay_in ... ok test model::tests::enhance_pickup_dropoff::stay_in_different_stop ... ok test model::tests::enhance_pickup_dropoff::stay_in_different_stop_overlapping_time ... ok test model::tests::enhance_pickup_dropoff::stay_in_same_stop ... ok test model::tests::enhance_pickup_dropoff::block_id_on_overlapping_calendar_forbidden_pickup ... ok test model::tests::enhance_pickup_dropoff::block_id_on_non_overlaping_calendar_with_overlaping_stops ... ok test model::tests::enhance_pickup_dropoff::block_id_on_non_overlaping_calendar_ko ... ok test model::tests::enhance_pickup_dropoff::block_id_on_overlapping_calendar_ok ... ok test model::tests::enhance_pickup_dropoff::forbidden_drop_off_should_be_kept ... ok test model::tests::enhance_route_directions::generate_route_direction ... ok test model::tests::enhance_route_names::do_not_generate_route_name_when_stops_names_are_empty ... ok test model::tests::enhance_trip_headsign::enhance ... ok test model::tests::enhance_route_names::generate_destination_id ... ok test model::tests::enhance_route_names::generate_route_name ... ok test model::tests::enhance_route_names::most_frequent_origin_destination ... ok test model::tests::enhance_route_names::same_frequency_same_size_stop_area_then_first_aphabetical_order ... ok test model::tests::enhance_route_names::same_frequency_then_biggest_stop_area ... ok test model::tests::enhance_trip_headsign::enhance_when_string_empty ... ok test gtfs::read::tests::prefix_on_all_pt_object_id ... ok test model::tests::update_stop_area_coords::update_coords ... ok test netex_france::calendars::tests::valid_day_bits::empty_validity_pattern ... ok test model::tests::update_stop_area_coords::update_coords_on_not_referenced_stop_area ... ok test netex_france::calendars::tests::valid_day_bits::not_successive_dates ... ok test netex_france::calendars::tests::valid_day_bits::only_one_date ... ok test netex_france::calendars::tests::valid_day_bits::successive_dates ... ok test netex_france::route_points::tests::build_route_points::circular_vehicle_journey ... ok test netex_france::route_points::tests::build_route_points::extended_vehicle_journey ... ok test netex_france::route_points::tests::build_route_points::forked_journey_pattern ... ok test netex_france::route_points::tests::build_route_points::intertwined_stops ... ok test netex_france::route_points::tests::build_route_points::in_between_stop ... ok test netex_france::route_points::tests::build_route_points::same_journey_pattern ... ok test netex_utils::tests::frame_type::parse_frame_type ... ok test netex_utils::tests::frame_type::parse_invalid_frame_type ... ok test netex_utils::tests::get_only_frame::multiple_frames ... ok test netex_utils::tests::get_only_frame::no_frame ... ok test netex_utils::tests::get_only_frame::one_frame ... ok test netex_utils::tests::parse_frames_by_type::some_frame ... ok test netex_utils::tests::parse_frames_by_type::unknown_frame ... ok test netex_utils::tests::value_in_keylist::has_value ... ok test netex_utils::tests::value_in_keylist::no_key_found ... ok test netex_utils::tests::value_in_keylist::no_keylist_found ... ok test netex_utils::tests::value_in_keylist::no_value_found ... ok test ntfs::read::tests::company_object_codes ... ok test ntfs::read::tests::ntfs_stop_times_precision ... ok test netex_france::stops::tests::valid_impaired_access::test_impaired_access_unknown ... ok test ntfs::read::tests::read_stop_points_with_no_parent ... ok test ntfs::tests::admin_stations_serialization_deserialization ... ok test netex_france::offer::tests::journey_patterns_with_different_number_stop_times ... ok test ntfs::tests::calendar_serialization_deserialization ... ok test netex_france::offer::tests::same_journey_pattern ... ok test ntfs::tests::commercial_modes_serialization_deserialization ... ok test ntfs::tests::companies_serialization_deserialization ... ok test ntfs::tests::contributors_serialization_deserialization ... ok test netex_france::stops::tests::valid_impaired_access::test_impaired_access_true ... ok test netex_france::offer::tests::journey_patterns_with_different_stop_time_properties ... ok test ntfs::tests::fares_v1_serialization_deserialization ... ok test ntfs::tests::datasets_serialization_deserialization ... ok test ntfs::tests::equipments_serialization_deserialization ... ok test ntfs::tests::feed_infos_serialization_deserialization ... ok test ntfs::tests::geometries_serialization_deserialization ... ok test ntfs::tests::networks_serialization_deserialization ... ok test ntfs::tests::od_fares_v1_serialization_deserialization ... ok test ntfs::tests::lines_serialization_deserialization ... ok test ntfs::tests::physical_modes_serialization_deserialization ... ok test ntfs::tests::prices_v1_serialization_deserialization ... ok test netex_france::stops::tests::valid_impaired_access::test_impaired_access_false ... ok test ntfs::tests::routes_serialization_deserialization ... ok test ntfs::tests::ticket_prices_serialization_deserialization ... ok test ntfs::tests::stops_serialization_deserialization ... ok test ntfs::tests::ticket_use_perimeters_serialization_deserialization ... ok test ntfs::tests::ticket_use_restrictions_serialization_deserialization ... ok test netex_france::stops::tests::valid_impaired_access::test_impaired_access_partial ... ok test ntfs::tests::ticket_uses_serialization_deserialization ... ok test ntfs::tests::tickets_serialization_deserialization ... ok test objects::tests::approx_distance ... ok test objects::tests::orthodromic_distance ... ok test objects::tests::rgb_deserialization_with_bad_number_of_digits ... ok test objects::tests::rgb_deserialization_with_too_big_color_hex ... ok test objects::tests::rgb_serialization ... ok test objects::tests::rgb_good_deserialization ... ok test objects::tests::time_serialization ... ok test objects::tests::time_deserialization ... ok test ntfs::tests::trip_properties_serialization_deserialization ... ok test ntfs::tests::transfers_serialization_deserialization ... ok test read_utils::tests::path_file_handler ... ok test utils::tests::deserialize_decimal::negative_decimal ... ok test utils::tests::deserialize_decimal::not_a_number ... ok test utils::tests::deserialize_decimal::positive_decimal ... ok test utils::tests::serde_currency::test_de_invalid_currency_code ... ok test utils::tests::serde_currency::test_ser_invalid_currency_code ... ok test utils::tests::serde_option_string::with_empty_string ... ok test utils::tests::serde_option_string::with_string ... ok test utils::tests::serde_currency::test_serde_valid_currency_code ... ok test utils::tests::serde_option_string::without_field ... ok test validity_period::tests::set_validity_period::no_existing_validity_period ... ok test read_utils::tests::zip_file_handler ... ok test validity_period::tests::set_validity_period::with_extended_validity_period ... ok test validity_period::tests::set_validity_period::with_included_validity_period ... ok test vptranslator::tests::bound_cut ... ok test vptranslator::tests::bound_compression ... ok test vptranslator::tests::empty_vp ... ok test vptranslator::tests::bound_cut_one_day ... ok test vptranslator::tests::one_week ... ok test vptranslator::tests::mwtfss_mttfss_mtwfss ... ok test vptranslator::tests::may2015 ... ok test vptranslator::tests::one_week_partial ... ok test vptranslator::tests::only_first_day ... ok test vptranslator::tests::only_one_monday ... ok test vptranslator::tests::only_one_sunday ... ok test vptranslator::tests::only_one_tfss ... ok test ntfs::tests::vehicle_journeys_and_stop_times_serialization_deserialization ... ok test vptranslator::tests::only_one_thursday ... ok test vptranslator::tests::partial_one_week ... ok test vptranslator::tests::partial_one_week_partial_with_nb_partial_6 ... ok test vptranslator::tests::partial_one_week_partial_with_nb_partial_7 ... ok test vptranslator::tests::partial_one_week_partial_with_nb_partial_8 ... ok test vptranslator::tests::prev_monday_from_monday ... ok test vptranslator::tests::prev_monday_from_sunday ... ok test vptranslator::tests::prev_monday_from_thursday ... ok test validity_period::tests::compute_dataset_validity_period::test_compute_dataset_validity_period_with_only_one_date ... ok test vptranslator::tests::t_w_t ... ok test vptranslator::tests::three_complete_weeks ... ok test vptranslator::tests::three_mtwss_excluding_one_day ... ok test vptranslator::tests::three_mtwss_including_one_day ... ok test validity_period::tests::compute_dataset_validity_period::test_compute_dataset_validity_period ... ok test ntfs::tests::comments_codes_object_properties_serialization_deserialization ... ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much, very nice to see people contributing to this project. I've noticed 2 tiny things that could be improved (see comments).
And for the tests, do not care too much about it, you're probably missing some local setup for the feature xmllint
(see the CONTRIBUTING.md
). In your case, you'll probably be fine to only run feature proj
(cargo test --features proj
) instead of all features (cargo test --all-features
). Note that the CI is happy anyway so you indeed didn't break anything 😉
src/utils.rs
Outdated
wkt.items | ||
.pop() | ||
.unwrap() | ||
.try_into() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to replace it with this
let wkt = wkt::Wkt::from_str(&s).map_err(serde::de::Error::custom)?;
use std::convert::TryInto;
wkt.try_into().map_err(serde::de::Error::custom)
README.md
Outdated
@@ -53,7 +53,7 @@ So it must be installed on the system to compile and use those crates. | |||
### [PROJ] for binaries | |||
|
|||
Using the [`proj` crate] requires some system-dependencies installation.\ | |||
The version `7.1.0` of [PROJ] is needed (used and tested by maintainers). | |||
The version `7.2.1` of [PROJ] is needed (used and tested by maintainers). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more a self-reminder: We have to be consistent about the version all over the codebase (dockerfile, makefile, CI). So there is a bit more work to be done on our side, or we may stay on 7.1.0
everywhere (let us know if you noticed some things required from 7.2
).
Maybe we can improve doc on that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if there are any relevant behavioral changes.
I updated this naively, just to conform to the version of proj that the crate expects: https://github.com/georust/proj/blob/0.22.0/proj-sys/build.rs#L11
Actually - there's one more bit of proj related documentation I'd like to update... |
re: ec20c88 Proj now builds from source automatically if a recent enough installation wasn't found. I updated the docs and the make task accordingly. 🚨 However, I haven't tested it, as I'm not running linux. I would appreciate if someone on that system could give it a go: |
Ok, I think this is ready for re-review. CI is failing w/ clippy. But reviewing the suggested changes, they all appear to be unrelated to changes in this PR. |
Yes, we saw this auto-build feature in |
As for |
#744 has been merged. You might need to rebase and |
ec20c88
to
d97b9e1
Compare
Your PR is good now, thank you very much. However, we'll wait for merging it because we still have some actions to take on our side before this is merged:
I'm approving the PR, but I'll put a Thanks again 😍 |
Sounds good.
In case it's useful to you, we went the route of using our own CI container to speed up our builds: this container builds, but doesn't install libproj. This container is used by multiple other containers: an example of a ci container which uses it: |
Yes, that's pretty much what we're doing, just using a custom Thanks for the feedback. |
The containers are public, so you are of course welcome to use it directly or as a reference for your own Dockerfile, but be advised that they are not intended to be "supported" and come with no warranties. =) |
For information (and to have it more accessible): please note that we are still planning on merging it. And we keep on tracking what's left to do inside this comment: #743 (comment) |
I opened for PR to update the dependencies: |
d97b9e1
to
417f632
Compare
Hello @michaelkirk We would like the libproj bump to be a manual conscious step (not silent during a proj crate bump) because:
EDIT: found acceptable libproj already installed at: "/usr/lib"
thread 'main' panicked at '`libclang` function not loaded: `clang_Cursor_Evaluate`. This crate requires that `libclang` 3.9 or later be installed on your system. |
I understand why it could be helpful to have a single version of the crate be compatible with multiple versions of libproj, but currently this is not possible - the proj crate only supports only one version of proj, so there is no way to "pin" it to an older version other than to pin your crate. It's possible we could support this one day - e.g. like the gdal crate does.
Yikes! Can you open an issue on the proj repo with any details?
Despite the name, the
re: pkg-config I'm not 100%, but I suspect pkg-config will still be needed to be sure the version that exists is compatible with the crate. Even if we support a window of libproj versions, it's vanishingly unlikely that we'll support all versions of proj ever released. We need to have some way of checking version constraints and pkg-config does that for us. Can you elaborate on your concern? re: clang I don't think we have any hard dep on clang. Can you point out where it is if I'm mistaken? |
First, thank you very much for your replies, helped a lot to move on here 🙏, very clear (will reply only on pending issues).
As for the found acceptable libproj already installed at: "/usr/lib"
thread 'main' panicked at '`libclang` function not loaded: `clang_Cursor_Evaluate`. This crate requires that `libclang` 3.9 or later be installed on your system. Did not dig much on that as it's easy and reasonable to provide
I'm not sure it's worth opening an issue as far as we went on the subject, but I'll give the details here, let me know if you still think we should. We were using libproj7.2.1 with all the deps of It's unlikely that we spend much more time on this if our tests do not suspiciously move again. Once more, thanks! |
Merged!
I'm not sure what part of Since there isn't a way to produce a problem with the current releases, I don't think filing an issue would be a very good use of anyone's time. It is strange though. |
Thanks for the merge, we will wait for https://github.com/CanalTP/ci-images/pull/28 to have CI working, but I don't see any blocker now 🎉 |
Also: * document libproj deps for maintenance * bump other deps
84445f8
to
b1910e0
Compare
I work on some of the libraries you use and wanted to see how some recent changes affect our users.
Thought I might as well share the changes in case you'd like to update.