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

12 task 2b primary locations work #15

Merged
merged 49 commits into from
Jul 31, 2024
Merged

Conversation

Hussein-Mahfouz
Copy link
Collaborator

When complete, this PR will allow us to assign people to actual primary locations. See #12 for details

@Hussein-Mahfouz Hussein-Mahfouz linked an issue Apr 17, 2024 that may be closed by this pull request
@Hussein-Mahfouz
Copy link
Collaborator Author

A couple of errors thrown by ruff that I think we could relax:

G004 Logging statement uses f-string
EM101 Exception must not use a string literal, assign to variable first

What do you think @sgreenbury ?

@sgreenbury
Copy link
Collaborator

@Hussein-Mahfouz: this looks great, I have updated the notebook and scripts the following changes:

  • Update region to Leeds only
  • Transform CRS from 27700 to 4326 for centroids no longer included with SPC
  • Update dep for uatk-spc
  • Remove recoding at start of notebook since already recoded (in second script?)
  • Fix remaining lints

It might be worth refactoring the scripts more extensively so their purpose is solely for processing the data (e.g. remove plots and other interactive aspects), but I think this would be better to look at in a new issue/PR.

@Hussein-Mahfouz
Copy link
Collaborator Author

Nice one! Happy to merge this :)

It might be worth refactoring the scripts more extensively so their purpose is solely for processing the data (e.g. remove plots and other interactive aspects), but I think this would be better to look at in a new issue/PR.

Yes agreed let's keep this one for a different PR. As discussed, the contents of the scripts might change so that we are dealing with all primary trips together

Copy link
Collaborator

@sgreenbury sgreenbury left a comment

Choose a reason for hiding this comment

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

Thanks @Hussein-Mahfouz, will go ahead and merge this now.

@sgreenbury sgreenbury merged commit ac834c1 into main Jul 31, 2024
4 checks passed
@sgreenbury sgreenbury deleted the 12-task-2b-primary-locations-work branch July 31, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Task 2b: Primary Locations (Work)
2 participants