Update packages and methods for 2024 #384
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 4.4.9 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,12 +5,17 @@ echo. | |
| set "DOCKERUSER=globalhealthyliveablecities" | ||
| set "PACKAGE=global-indicators" | ||
| for /f "usebackq" %%x in ("%CD%\..\.ghsci_version") do set VERSION=%%x | ||
| echo %PACKAGE% version %VERSION% | ||
| :: login and remove any existing containers or images | ||
| docker login | ||
|
|
||
| :: build test image and export the conda env to yml | ||
| docker build -t %DOCKERUSER%/%PACKAGE% . | ||
| docker run --rm -it --shm-size=2g --net=host -v "%CD%":/home/ghsci %DOCKERUSER%/%PACKAGE% /bin/bash -c "pip list --format=freeze > ./requirements.txt" | ||
|
There was a problem hiding this comment. I think this is because this 'docker run' command is launching the container using a historical snippet of code, overriding the deafult size that was (or perhaps was?) 64mb, since that would not be sufficient to adequately/reliably analyse most cities. However --- good point that in that particular context, setting shm-size isn't necessary, since all that the code is doing there is launching the container to grab a copy of the specific package versions that were installed and output shm_size is set for analysis purposes in the compose yml now --- still to 2gb, although, for users with sufficient memory analysing larger cities, they may want/need to increase that: Re: the batch file for building the docker image, I'll make a note to edit this later in the week. If you think the setting is no longer required in the compose file, feel free to let me know. |
||
|
|
||
| :: built multi-platform image | ||
| docker buildx create --use | ||
| docker buildx build --platform=linux/amd64,linux/arm64 -t %DOCKERUSER%/%PACKAGE%:v%VERSION% . | ||
|
|
||
| :: get the package version, tag the image with it, then push to hub | ||
| echo %PACKAGE% version %VERSION% | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,16 +3,21 @@ channels: | |
| - conda-forge | ||
| dependencies: | ||
| - python | ||
| - osmnx=1.5.* | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. We could give it a go --- it may have been I pinned to 1.5.* to avoid refactoring code due to other changes in dependencies (I tried to make software as new as possible, while re-writing code as least as possible). As above, I'll try the update and see how that goes with other dependencies/refactoring later in the week. There was a problem hiding this comment. Reflecting on this, your time is best served holding off on this one for now. We're working on some deprecations in advance of OSMnx v2 later this year. If it will require some refactoring in our codebase here to upgrade anyway, then we should just hold off until those deprecations for the OSMnx v2 API are all in place in a couple of months. That is, if you have to do some refactoring anyway, then might as well wait to just do it all at once rather than piecemeal. There was a problem hiding this comment. Sounds good @gboeing; thanks for heads up. I am sure there will be more patches requiring updates here by that stage in any case, so will be a good opportunity to review things at that point and see if we can better leverage OSMnx functionality and related packages and drop some other dependencies perhaps. |
||
| - pandas=2.1.* | ||
| - fpdf2=2.7.* | ||
| - sqlalchemy=1.4.* | ||
| - nicegui=1.4.13 | ||
|
There was a problem hiding this comment. A lot of these deps are pinned down to the patch. Can we pin to the minor version instead? Patches should always be compatible across a minor version, and it would allow installers to bring in any important fixes. There was a problem hiding this comment. Nicegui is under rapid development (which is good) and they seem to change syntax regularly (can be a bit frustrating). I guess I was being explicit about which version I had designed our code to work with. For other packages, I was following dependabot recommendations. But having said that, if you think that would be better practice, lets give it a go. As above, I'll aim to build another version of the image later in the week in case there is further feedback. There was a problem hiding this comment. @gboeing I just had a quick go at rebuilding the image removing the minor version pins; as expected, the web app didn't function with this modification (it doesn't load, due to a bug introduced in a recent minor patch zauberzeug/nicegui#2641 ). Its possible other incompatibilities / syntax changes could have been introduced too by one or other packages (even if they shouldn't). But the current selection of versions works for the Docker image according to my tests. Given the current 4.4.9 build works, and we'll do a further environment update in a few months with OSMnx 2.0, I'm thinking the following:
then we'll look for a more thorough update of packages/environment and refactor code and do a 5.0 release then, perhaps? For now, are you okay with the merging of this branch as is? There was a problem hiding this comment. Yes sounds like a reasonable plan. |
||
| - cryptography=42.0.2 | ||
| - requests=2.31.* | ||
| - tornado>=6.3.3 | ||
| - fonttools=4.43.0 | ||
| - jupyterlab=4.0.12 | ||
| - jupyter-lsp=2.2.2 | ||
| - urllib3=2.0.7 | ||
| - pillow=10.2.0 | ||
| - pip>=24.* | ||
| - pyrosm=0.6.2 | ||
| - openpyxl | ||
| - babel | ||
| - cartopy | ||
|
|
@@ -27,4 +32,3 @@ dependencies: | |
| - tqdm | ||
| - pyyaml | ||
| - websockets | ||
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.
Do we need to pin the version number that precisely? Can we just use
latesthere?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.
Hi @gboeing, that's a good question -- perhaps we can. From memory, the reason I did a specific version at the time was, that was the latest release that supported Arm64. Previously, I had used the 'alpine' image, which is smaller size, but doesn't reliably support arm64 (mostly only amd64). Perhaps that is why the alpine image is half the size though. If you believe the 'latest' miniconda3 reliably supports arm64, and using 'latest' is unlikely to introduce incompatibilities, I can't see why not (they just released an updated version 3 days ago, so already advanced since pull request lodged).
I'll look to updating this after I hear back comments from others later in the week, in case there are other things to shift.