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

290 docs overall tp pipeline #292

Merged
merged 9 commits into from
Jun 14, 2024
Merged

290 docs overall tp pipeline #292

merged 9 commits into from
Jun 14, 2024

Conversation

ethan-moss
Copy link
Collaborator

Description

Fixes #290 - adds a new explanation page covering the end-to-end transport performance pipeline.

Motivation and Context

Add an explanation page demonstrating, at a top-level, how transport_performance can be used to analyse transport networks.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

New documentation: All additions and changes render as expected locally.

Test configuration details:

  • OS: macOS
  • Python version: 3.9.13
  • Java version: N/A
  • Python management system: conda

Advice for reviewer

This branch is not yet up to date with main (likely to be a merge conflict in .gitignore. I'm happy to resolve this once #291 is merged to dev - just let me know.

Checklist:

  • My code follows the intended structure of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
    • Although, N/A for this PR.
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules:

Additional comments

None.

@ethan-moss ethan-moss added the documentation Improvements or additions to documentation label Jun 12, 2024
@ethan-moss ethan-moss added this to the Version 1 Release milestone Jun 12, 2024
@ethan-moss ethan-moss linked an issue Jun 12, 2024 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.13%. Comparing base (2ad9ec3) to head (b96b68a).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #292   +/-   ##
=======================================
  Coverage   98.13%   98.13%           
=======================================
  Files          21       21           
  Lines        1927     1927           
=======================================
  Hits         1891     1891           
  Misses         36       36           
Flag Coverage Δ
unittests 98.13% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@r-leyshon r-leyshon left a comment

Choose a reason for hiding this comment

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

Hi @ethan-moss, I've coughed up a bunch of commits which most were straight forward, apart from commit 7b1874a which like yesterday's PR, is commented as being more opinionated than the others. But like yesterday, happy to be disagreed with, in which case feel free to revert lines that you don't agree to.

A few things to note with this one, but this hopefully does not represent a great deal of work:

  1. I noticed an unresolved link warning when rendering the site. It was complaining about a link to validate_osm in the api reference. After re-running quartodoc build this went away, but I noticed a very minor typo in the osm tutorial. So that's been plopped at the end of this review too.
  2. The flowchart diagram is a little blurry when rendered for me, I'll attach what I see here for your consideration, though also noting that it actually looks a bit better in my screengrab than on my screen for some reason. This is probably a compromise between file size and getting a large enough diagram to be accessible. Preferably, I would recommend using a mermaid markdown cell to bring the flowchart into version control, but am unsure if there was a reason why you didn't originally do it that way.
image
  1. I didn't understand the left hand side of the flow chart. I wonder if people will get confused that the urban_centre is responsible for the GTFS & OSM cropping, rather than the user-defined BBOX. I felt it might be misconstrued as the population, GTFS and OSM processing occurring in parallel. As the flowchart is useful learning aid, I would recommend simplifying it further still to a linear diagram.

But other this I think the page does a great job of summarising the end to end process.

@r-leyshon r-leyshon merged commit 3fa154e into dev Jun 14, 2024
8 checks passed
@r-leyshon r-leyshon deleted the 290-docs-overall-tp-pipeline branch June 14, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs: overall transport performance pipeline
3 participants