-
Notifications
You must be signed in to change notification settings - Fork 4
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
Python 3.7 & 3.11 support #35
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes and explanation seem fine, but I'm guessing you did a bunch of verification of OSMOX with these modified dependencies in both a 3.7 and a 3.11 environment, and I need to see the details and outcome of that. With a change like this, the assurance that we've not broken anything is 95% of what I'm looking for.
Can you describe what you did and how? Include copy/pasted shell sessions, or screenshots, that kind of thing. Maybe you set up a clean virtual env for each Python version, installed with the new dependencies, then ran all the tests successfully? That would be a great thing to see. Showing me exactly what you did and saw beats telling me.
As far as the question about pinning to a specific version versus using looser version constraints, we would generally pin everything for an application, (because it's fair to assume that it will be running in a deterministic environment - think a dedicated virtual env), but take a looser approach with a library (because our library will be installed into the user's environment alongside a bunch of other libraries that we know nothing about, making it a non-deterministic environment and thus more prone to "dependency hell" type version clashes). You can see that with the geopandas>=0.9.0
dependency.
The downside is that too loose a set of version constraints can make pip install
run unusably slowly as it investigates every permutation of versions. In this case, I don't think it matters whether we pin Osmium or not; if we can get away with >=
, let's do that.
Finally, I've added a closes
comment to the PR. When your PR addresses a particular GitHub issue, you can link the two so that the issue will be closed automatically when the PR is merged.
Thanks Michael - that makes sense about the application vs. library version constraints. I've unpinned osmium now to Steps were:
|
Are you deliberately not merging this @val-ismaili , or did you forget about it? |
Closes #32
Based on @mfitz issue here, these changes to the requirements.txt seem to enable osmox to work in both a Python 3.7 and 3.11 in virtual environments on my machine. I've had seperate issues relating to me running this on an M1, so would be useful to see if others are able to run this ok with both python versions.
I originally just left the option as >= for both packages as below and this worked
Assuming that it would be better to pin the osmium & pyproj to specific versions (?) I've found that
osmium==3.4.0
is the crossover for support on both so I've changed it to that for now. The pyproj seems to need to stay with this>=
flexibility though.