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

osmox #40

Merged
merged 8 commits into from
Nov 7, 2024
Merged

osmox #40

merged 8 commits into from
Nov 7, 2024

Conversation

Hussein-Mahfouz
Copy link
Collaborator

Related to #19 and #39. We can either:

  • implement logic in this comment
  • point the user to osmox and tell them where to place the output file once they've created it

@sgreenbury
Copy link
Collaborator

sgreenbury commented Oct 18, 2024

Remaining tasks:

  • Update the output format to geoparquet
  • Update the output path to interim/osmox and change the downstream scripts to point to this
  • Check minimum python version issue with 3.12
  • Ensure the output from pyrosm matches the region config name

@sgreenbury
Copy link
Collaborator

Adding the deps mentioned for osmium fixes the CI for Python 3.12 (b54cff0)

@Hussein-Mahfouz
Copy link
Collaborator Author

Do these dependencies need to be added to the poetry lock file?

@sgreenbury
Copy link
Collaborator

Do these dependencies need to be added to the poetry lock file?

Since they're external system deps to python I don't think we're able to install with poetry though we can add this to a requirements section in the docs and link to the osmium docs

@sgreenbury
Copy link
Collaborator

Merging this now with (the output file from pyrosm can be passed directly to osmox (1504c59).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps we can move these osmox notes to a section in the documentation as part of #63?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does it make sense to add the notes as a subsection under the Installation section of the main README? If that sounds good, do you mind doing it? I think you're better placed to add the notes than I am.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good, I'll go ahead and add these notes to the docs as part of #63. I think we can have a section on configuration of the pipeline and we can include a subsection on osmox and mention the default config file JSON (config_osmox.json) along with the descriptions here.

@sgreenbury sgreenbury merged commit ca181c5 into main Nov 7, 2024
4 checks passed
@sgreenbury sgreenbury deleted the osmox branch November 7, 2024 10:45
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.

2 participants