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

Update scripts dependencies #179

Merged
merged 5 commits into from
Jul 27, 2023
Merged

Update scripts dependencies #179

merged 5 commits into from
Jul 27, 2023

Conversation

kylebarron
Copy link
Collaborator

@kylebarron kylebarron commented Jul 27, 2023

Closes #178

Now

poetry install
poetry run python generate_example.py

works on my machine

@kylebarron
Copy link
Collaborator Author

This fails the tests because it uses a later version of GeoPandas/Pyproj and writes PROJJSON with a schema of v0.6. I created #180

@tschaub
Copy link
Collaborator

tschaub commented Jul 27, 2023

This should probably be a dedicated ticket, but I've run into this same problem that the CI job is hitting with the projjson schema dependency. It is not a real problem until a version of projjson is released that is incompatible with https://proj.org/schemas/v0.5/projjson.schema.json. But it looks like we need to relax the test_json_schema.py so it isn't as strict about the version that is included in the generated projjson.

@kylebarron
Copy link
Collaborator Author

But it looks like we need to relax the test_json_schema.py so it isn't as strict about the version that is included in the generated projjson.

It looks like pytest actually succeeds (you can run poetry run python generate_example.py && poetry run pytest locally; it works for me). It's these lines in the CI:

# Assert that the version number and file metadata are up to date
# Allow for differences in example.parquet
git restore example.parquet
git diff
test -z "$(git status --porcelain)"

@tschaub
Copy link
Collaborator

tschaub commented Jul 27, 2023

Just saw the same after re-reading. Yes, test -z "$(git status --porcelain)" is the problem.

@tschaub
Copy link
Collaborator

tschaub commented Jul 27, 2023

Woohoo. Thanks, @kylebarron

@tschaub
Copy link
Collaborator

tschaub commented Jul 27, 2023

I'll merge this so we can proceed with the re-release. Hope that sounds ok. Thanks for fixing it up.

@tschaub tschaub merged commit dc941c0 into main Jul 27, 2023
3 checks passed
@jorisvandenbossche jorisvandenbossche deleted the kyle/update-scripts-dependencies branch August 7, 2023 16:58
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.

Simplify or remove script dependencies
2 participants