-
Notifications
You must be signed in to change notification settings - Fork 0
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
Read dataset #3
base: main
Are you sure you want to change the base?
Read dataset #3
Conversation
=== Do not change lines below === { "chain": [], "cmd": "poetry run python3 scripts/read_all_domains.py", "dsid": "934d913e-8268-4342-aa0c-3702ee516d10", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^
=== Do not change lines below === { "chain": [], "cmd": "poetry run python3 scripts/read_all_domains.py", "dsid": "934d913e-8268-4342-aa0c-3702ee516d10", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^
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.
most looks good, but the make targets don't work for me because of locking issues. And some more smaller comments.
Makefile
Outdated
@@ -82,3 +82,8 @@ virtual-environment: ## update virtual environment, create a new one if it does | |||
download_all_domains: | |||
# downloads and stages (datalad save) all available data | |||
datalad run poetry run python3 scripts/download_all_domains.py | |||
|
|||
.PHONY: read_latest_data |
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.
Will this fail is you run it when not connected to the internet? I usually don't link read to download so data is not downloaded again if I have to read it again because of a processing code change
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.
OK, realized it doesn't fail, because make recognizes that the files are there. But it will not check for a new version.
This make target says "nothing to be done" after changing the data reading code. This should be changed, so we can re-read data after making changes to the code.
@@ -0,0 +1,8 @@ | |||
"""Read the latest release of all available domains.""" |
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.
More general comment: I think it would e good to have an option to re-read older data as well,e.g. if we read additional data or variables.
from pathlib import Path | ||
|
||
|
||
def get_root_path(root_indicator: str = ".git") -> Path: |
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.
nice to use an indicator file. This avoids setting an environment variable as in the UNFCCC_non-annexI_data repo.
Makefile
Outdated
@@ -82,3 +82,8 @@ virtual-environment: ## update virtual environment, create a new one if it does | |||
download_all_domains: |
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.
For me download_all_domains
tries to read the data into primap2 format. This is not what I would intuitively expect it to do (also it fails)
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.
Fails because the target files are locked.
dimensions: | ||
'*': | ||
- time | ||
- category (FAOSTAT) |
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.
Category is to be changed, after the category tree is done?
=== Do not change lines below === { "chain": [], "cmd": "python3 scripts/download_all_domains.py", "dsid": "934d913e-8268-4342-aa0c-3702ee516d10", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [ "downloaded_data" ], "pwd": "." } ^^^ Do not change lines above ^^^
=== Do not change lines below === { "chain": [], "cmd": "python3 scripts/download_all_domains.py", "dsid": "934d913e-8268-4342-aa0c-3702ee516d10", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [ "downloaded_data" ], "pwd": "." } ^^^ Do not change lines above ^^^
=== Do not change lines below === { "chain": [], "cmd": "python3 scripts/download_all_domains.py", "dsid": "934d913e-8268-4342-aa0c-3702ee516d10", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [ "downloaded_data" ], "pwd": "." } ^^^ Do not change lines above ^^^
# Conflicts: # src/faostat_data_primap/download.py
Switch to doit
…aset # Conflicts: # poetry.lock # pyproject.toml # requirements.txt
Description
Checklist
Please confirm that this pull request has done the following:
changelog/
Notes