Bypass external urls #1257
Bypass external urls #1257
Conversation
|
If you had time @ekatef , it would be great to review this PR and potentially merge. |
Perfect, fantastic news!! Thank you so much for tackling that. Reviewing 😄 |
There was a problem hiding this comment.
Perfect @davide-f! That is fantastic news that you have managed to make CI work, and geoboundaries seem to be a good replacement 😄
Have added a couple of comments which mainly relate to documenting the changes. Could you please to have a look? Happy to merge afterwards.
configs/bundle_config.yaml
Outdated
| - data/gadm/gadm41_NGA/gadm41_NGA.gpkg | ||
| - data/gadm/gadm41_BEN/gadm41_BEN.gpkg | ||
| - data/gadm/gadm36_NGA/gadm36_NGA.gpkg | ||
| - data/gadm/gadm36_BEN/gadm36_BEN.gpkg |
There was a problem hiding this comment.
As far as I remember, gadm41 is needed for the power-only model, while gadm36 is being used by the cross-sectoral part, right? It would be nice to have a comment on that
There was a problem hiding this comment.
Not sure it is so relevant to add these details; we have no comments on the other pieces, but I can add the lines
configs/bundle_config.yaml
Outdated
| - data/gadm/gadm41_BWA/gadm41_BWA.gpkg | ||
| - data/gadm/gadm36_BWA/gadm36_BWA.gpkg |
There was a problem hiding this comment.
Same comment as above: it would be really great to explain a reason why we need two sources for GADM
configs/bundle_config.yaml
Outdated
|
|
||
| # # tutorial bundle specific for Nigeria and Benin only, without gadm-like data | ||
| # bundle_tutorial_NGBJ: |
There was a problem hiding this comment.
Probably, it would be handy to have a short explanation on the reason to comment-out this part with a link to #1258
| co2limit = float(m[0]) * snakemake.params.electricity["co2base"] | ||
| co2limit = float(m[0]) * float(snakemake.params.electricity["co2base"]) | ||
| logger.info("Setting CO2 limit according to wildcard value.") | ||
| else: | ||
| co2limit = snakemake.params.electricity["co2limit"] | ||
| co2limit = float(snakemake.params.electricity["co2limit"]) |
There was a problem hiding this comment.
My guess is that these changes in prepare_network have been needed to test the workflow properly. Can you please confirm that? 🙂 (apologies for an idiotic question, just double-checking that the changes are intentional and exactly as they are supposed to be)
There was a problem hiding this comment.
Today there has been a temporary issue with the automatic download of emission data; because of that I disabled that and I had issues as the float(..) was needed to make it work.
It was a minor thing and decided to add it as it is minor
Closes #1256
Changes proposed in this Pull Request
Checklist
envs/environment.yamlanddoc/requirements.txt.config.default.yamlandconfig.tutorial.yaml.test/(note tests are changing the config.tutorial.yaml)doc/configtables/*.csvand line references are adjusted indoc/configuration.rstanddoc/tutorial.rst.doc/release_notes.rstis amended in the format of previous release notes, including reference to the requested PR.