Skip to content

Added bike theft data - #47

Merged
thesun7385 merged 7 commits into
bikespace:mainfrom
thesun7385:bike-thefts-data
Sep 13, 2026
Merged

Added bike theft data#47
thesun7385 merged 7 commits into
bikespace:mainfrom
thesun7385:bike-thefts-data

Conversation

@thesun7385

Copy link
Copy Markdown
Contributor

No description provided.

@tallcoleman tallcoleman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for putting this together - I’m very excited to get a working bike theft map up on the site!

I’ve left some review comments for things to look at. My goal is to help get this dataset into the pipeline as soon as possible to enable the frontend to go live as soon as it’s ready. I’ve organized my comments into two sections: “review now” should be resolved in this PR, and “review before frontend is published” can be done in a future PR. Most other comments should be optional.

Review now

  • The logic in bike-thefts-etl.py can be consolidated into update_bike_thefts.py so that you just have one script to call from the github action.
  • The preferred way to work with downloaded data is with pandas/geopandas. There are several utility functions in the shared folders that help with common tasks like saving a geopandas dataframe to a geojson file or downloading a geojson file from the open data portal.
  • The ideal output file format is .geojson; the MapLibre library that we use natively handles it well in an optimized way.
  • The output files should be saved in the folder bike_theft/ that the github action cherry picks over to the data branch on each run. Output files should generally not be saved to the main branch.
  • One exception to that: I think it would be helpful to save the documentation PDFs that are currently linked from the open data portal since they serve as a reference for some of the dicts used in the ETL processing.
  • A StatusManager instance should be added to track the metadata every time the data is updated. Lines 76-79, 93, and 133-139 in https://github.com/bikespace/parking-map-data/blob/main/src/bikespace_data/bicycle_network/update_cycling_network.py should give you a good example of usage.
  • Once you have a StatusManager instance set up, you can use it to check if the source dataset has been updated and skip the rest of the ETL if it has not. This check/skip should be added since the dataset is only updated annually, but the script will run every day.

Review before frontend is published

  • To prevent upstream changes to the data format from causing unanticpated script behaviour, I would reccommend adding validation for your source data using pandera (https://pandera.readthedocs.io/en/stable/). You can see a good example of this https://github.com/bikespace/parking-map-data/blob/main/src/bikespace_data/bicycle_network/update_cycling_network.py with how cycling_network_schema and cycling_network_schema_optional are used. This allows you to fail the processing script if e.g. an important column is removed or re-named in the source data and/or to log non-critical variances (e.g. a new location type is added).
  • I would recommend trying to add at least one or two unit tests. Doing test cases for one of the functions like normalize_color would be a relatively easy way to start. Running unit tests with coverage will help you catch subtle bugs in your logic flow, ensure that edge cases are handled properly, and gives a lot more confidence when you’re updating or building on the codebase later.

One other thing to think about for the future, is that each point represents the closest intersection to the location of the bike theft. It would be interesting to line this dataset up against the City’s intersection dataset (https://open.toronto.ca/dataset/intersection-file-city-of-toronto/) to see if there are any intersections where there are no bike thefts reported nearby.

Comment thread .github/workflows/run-update-sensor.yml Outdated
Comment thread src/bikespace_data/bike_thefts/bicycle-thefts.geojson Outdated
Comment thread src/bikespace_data/bike_thefts/stolen_bike_reports.json Outdated
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py Outdated
Comment thread src/bikespace_data/bike_thefts/bike-thefts-etl.py Outdated
Comment thread src/bikespace_data/bike_thefts/bike-thefts-etl.py Outdated
Comment thread src/bikespace_data/bike_thefts/bike-thefts-etl.py Outdated
Comment thread src/bikespace_data/bike_thefts/bike-thefts-etl.py Outdated
Comment thread src/bikespace_data/bike_thefts/bike-thefts-etl.py Outdated
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py Outdated
@tallcoleman

Copy link
Copy Markdown
Collaborator

Also FYI I ran our manual action to generate a preview: https://659fa6e7.bikespace-v2.pages.dev/stolen-history-map

@thesun7385 thesun7385 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tallcoleman I just updated the function to save the file in geojson, and I also revised the job update bike stolen history data as well. Feel free to have a look!

@tallcoleman tallcoleman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for making these updates! I've left some additional comments and once you've gone through those, we should be ready to merge.

I realized I should add better contributor documentation; I've opened pr 49 to do that and you can see the draft documentation here. Take a look through and let me know if there's anything there that could be improved?

For the final PR, please make sure that it only includes the source files and does not include the .geojson or status manager file outputs.

It's a bit tricky during development because you can't .gitignore the outputs (otherwise the data update action wouldn't work). If you want to be able to run the script and see the output files during development without having git track them and without breaking the update action, you can add the output folder to your local .git/info/exclude.

Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py Outdated
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py Outdated
Comment thread .github/workflows/run-update-sensor.yml Outdated
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py Outdated
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py Outdated
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py

@thesun7385 thesun7385 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tallcoleman Thank you for your prompt review. I just updated the file paths, git action, the save function. I also excluded unnecessary files that you mention earlier. Feel free to have a look!

@tallcoleman tallcoleman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for making those updates! Nearly there, just two blocking issues that need to be resolved:

  • fixOUTPUT_DIR and a bug you've helped us find with save_geo_output
  • remove output file src/bikespace_data/bike_thefts/source_files/bicycle-thefts_raw.geojson

Comment thread src/bikespace_data/bike_thefts/source_files/bicycle-thefts_raw.geojson Outdated
Comment thread src/bikespace_data/bike_thefts/README.md
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py Outdated
Comment thread src/bikespace_data/bike_thefts/update_bike_thefts.py

@thesun7385 thesun7385 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tallcoleman As promised, the file path was fixed. Feel free to take a look. I did add a new function to build additional description. Feel free to have a look!

@tallcoleman tallcoleman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for your hard work! This is now approved and ready to merge.

Please squash merge and make sure to add a descriptive message to the merge commit.

@tallcoleman tallcoleman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One last thing to fix and then you're good to merge

Comment thread .gitignore
@thesun7385

thesun7385 commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Bike theft dataset

Thank you @tallcoleman for reviewing.

  • Updated the ETL workflow (added the build‑description function and corrected file paths).
  • Updated geojson output to follow project's standards.
  • Updated the final file location for the update-bike-theft-data job in the run-update-sensor.yml
  • Fixed the final .gitignore by removing the bike_thefts paths from the root directory.
  • Included README.md for additonal details.

Merging Now!

@thesun7385
thesun7385 merged commit 846bf8b into bikespace:main Sep 13, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in BikeSpace Project Tracker Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants