Skip to content

Conversation

@landonreed
Copy link
Contributor

@landonreed landonreed commented Nov 30, 2020

Checklist

  • Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
  • Any modified or new methods or classes have helpful JSDoc and code is thoroughly commented
  • The description lists all applicable issues this PR seeks to resolve
  • The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

fix #627. Alternative approach to #628. This works with changes from conveyal/gtfs-lib#297

@codecov-io
Copy link

codecov-io commented Nov 30, 2020

Codecov Report

Merging #630 (893b789) into dev (b007168) will decrease coverage by 20.92%.
The diff coverage is 20.83%.

Impacted file tree graph

@@             Coverage Diff             @@
##              dev     #630       +/-   ##
===========================================
- Coverage   36.61%   15.68%   -20.93%     
===========================================
  Files         326      326               
  Lines       17782    16627     -1155     
  Branches     5397     5025      -372     
===========================================
- Hits         6510     2608     -3902     
- Misses       9786    11976     +2190     
- Partials     1486     2043      +557     
Flag Coverage Δ
end_to_end_tests ?
unit_tests 15.68% <20.83%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ib/editor/components/map/EditorMapLayersControl.js 0.00% <0.00%> (-44.00%) ⬇️
lib/editor/containers/ActiveGtfsEditor.js 0.00% <0.00%> (-66.67%) ⬇️
lib/gtfs/actions/shapes.js 0.00% <0.00%> (-11.54%) ⬇️
lib/gtfs/components/GtfsMap.js 0.00% <0.00%> (-46.72%) ⬇️
lib/gtfs/components/ShowAllRoutesOnMapFilter.js 0.00% <0.00%> (-40.91%) ⬇️
lib/gtfs/util/graphql.js 35.41% <ø> (+3.93%) ⬆️
lib/types/reducers.js 0.00% <ø> (ø)
lib/common/util/gtfs.js 73.68% <25.00%> (-12.99%) ⬇️
lib/editor/reducers/data.js 10.40% <28.57%> (-33.21%) ⬇️
lib/editor/actions/tripPattern.js 17.74% <38.46%> (+1.36%) ⬆️
... and 273 more

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 b007168...893b789. Read the comment docs.

Copy link
Contributor

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Please address merge conflicts and build failures...

Copy link
Contributor

@evansiroky evansiroky left a comment

Choose a reason for hiding this comment

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

  • Fix failing travis build
  • Fix merge conflicts

@evansiroky evansiroky removed their assignment Dec 1, 2020
@landonreed landonreed assigned evansiroky and unassigned landonreed Dec 2, 2020
@landonreed landonreed requested a review from evansiroky December 2, 2020 14:53
Copy link
Contributor

@evansiroky evansiroky left a comment

Choose a reason for hiding this comment

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

Approved, pending the changes in conveyal/gtfs-lib#297 percolating it's way through the pipeline.

Copy link
Contributor

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Looks good and seems to be working after regenerating editor snapshots.

@landonreed landonreed added the BLOCKED Blocked (waiting on another PR to be merged) label Dec 3, 2020
@landonreed
Copy link
Contributor Author

Marked as blocked until datatools-server has the latest gtfs-lib release.

@evansiroky
Copy link
Contributor

Something I just noticed... the Feed Version map still uses the raw GTFS shapes attributes to find the shapes:

export function toggleShowAllRoutesOnMap (namespace: string) {
return function (dispatch: dispatchFn, getState: getStateFn) {
let state = getState()
dispatch(updateRoutesOnMapDisplay(!state.gtfs.filter.showAllRoutesOnMap))
state = getState()
if (
state.gtfs.filter.showAllRoutesOnMap &&
!state.gtfs.shapes.fetchStatus.fetched &&
!state.gtfs.shapes.fetchStatus.fetching
) {
dispatch(fetchingShapes())
return dispatch(fetchGraphQL({query: shapes, variables: {namespace}}))
.then(data => {
const {patterns} = data.feed
const shapes = patterns.map(pattern => {
return pattern.shape.map(point => [
point.shape_pt_lat,
point.shape_pt_lon
])
})
dispatch(receiveShapes(shapes))
})
}
}
}

Can this be updated too?

@landonreed landonreed merged commit efeae58 into dev Dec 22, 2020
@landonreed landonreed deleted the refactor-route-layer branch December 22, 2020 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BLOCKED Blocked (waiting on another PR to be merged)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editor: Show routes on a map causes out of memory issues

5 participants