-
Notifications
You must be signed in to change notification settings - Fork 2
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
EEA reader #44
EEA reader #44
Conversation
…Data from data; have not tested that the rest of the reading works
Still to be done:
|
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.
Thanks for the implementation. Please use std-python libs or existing dependencies unless really required and discussed elsewhere.
I think metadata should be part of the data-download, not part of this reader. See the existing database at: /lustre/storeB/project/aerocom/aerocom1/AEROCOM_OBSDATA/EEA_AQeRep.v2/download
The test-database is much too big. Please shrink it to ~100k in total. Some files have more than 1MB.
with open("concentration.csv") as source: | ||
response = [] | ||
for line in source: | ||
words = line.split(",") |
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.
consider using csv
module with DictReader. Much easier to read line["Sampling Point Id"] than words[0].
separator seems to be ", "
PR metno/pyaerocom#1335 must also be merge before the reader works with aeroval |
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.
Looks very good. A small comment on tomli and python-version.
And a big question: The times in the parquet files are (EEA-specification) given in UTC+1, not UTC. pyaro expects times in UTC. Do you convert?
from pathlib import Path | ||
|
||
|
||
import tomli as tomllib |
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.
this should have a version test since tomllib is include >= python 3.11
if sys.version_info.minor >= 11: | ||
import tomllib | ||
else: | ||
import tomli as tomllib |
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.
if sys.version_info >= (3, 11):
import tomllib
else:
import tomli as tomllib
or more pragmatic:
try:
import tomllib
except ImportError:
import tomli as tomllib
No description provided.