Skip to content
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

First try at fixing col setup without start stop. Model validator for… #1212

Open
wants to merge 2 commits into
base: main-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions pyaerocom/colocation/colocation_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,16 @@ def validate_no_forbidden_keys(self):
if key in self.model_fields:
raise ValidationError

@cached_property
def basedir_logfiles(self):
p = Path(self.basedir_coldata) / "logfiles"
if not p.exists():
p.mkdir(parents=True, exist_ok=True)
return str(p)
return self
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why return self?


@model_validator(mode="after")
# @classmethod
def validate_start_stop_xand(self):
if not (self.start and self.stop):
if self.start or self.stop:
Comment on lines +472 to +473
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the nested if needed?
maybe this could be re-written as

        if not (self.start and self.stop) and self.start or self.stop:

or

        if bool(self.start) != bool(self.stop):

raise ValueError("Both start and stop need to be provided or both not provided.")

# return self
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the previous validator returns `self, why this one does not?


@model_validator(mode="after")
@classmethod
Expand All @@ -492,6 +496,13 @@ def validate_obs_config(cls, v: PyaroConfig):
cls.obs_id = v.name
return v

@cached_property
def basedir_logfiles(self):
p = Path(self.basedir_coldata) / "logfiles"
if not p.exists():
p.mkdir(parents=True, exist_ok=True)
Comment on lines +502 to +503
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no need for the if, the exist_ok=True parameter does the checking.
You can simple write:

        p.mkdir(parents=True, exist_ok=True)

return str(p)

def add_glob_meta(self, **kwargs):
"""
Add global metadata to :attr:`add_meta`
Expand Down
8 changes: 6 additions & 2 deletions pyaerocom/colocation/colocator.py
Original file line number Diff line number Diff line change
Expand Up @@ -914,8 +914,13 @@ def _infer_start_stop_yr_from_model_reader(self):
self.stop = last

def _check_set_start_stop(self):
if self.colocation_setup.start is None:
if self.colocation_setup.start is None and self.colocation_setup.stop is None:
self._infer_start_stop_yr_from_model_reader()
self.start, self.stop = start_stop(self.start, self.stop)
else:
self.start, self.stop = start_stop(
self.colocation_setup.start, self.colocation_setup.stop
)
if self.colocation_setup.model_use_climatology:
if self.colocation_setup.stop is not None or not isinstance(
self.colocation_setup.start, int
Expand All @@ -925,7 +930,6 @@ def _check_set_start_stop(self):
'climatology fields, please specify "start" as integer '
'denoting the year, and set "stop"=None'
)
self.start, self.stop = start_stop(self.colocation_setup.start, self.colocation_setup.stop)

def _coldata_savename(self, obs_var, mod_var, ts_type, **kwargs):
"""Get filename of colocated data file for saving"""
Expand Down
2 changes: 2 additions & 0 deletions tests/colocation/test_colocator.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def setup():
obs_id="AeronetSunV3L2Subset.daily",
obs_vars="od550aer",
start=2010,
stop=2011,
raise_exceptions=True,
reanalyse_existing=True,
)
Expand Down Expand Up @@ -180,6 +181,7 @@ def test_Colocator__coldata_savename(setup):
setup["model_name"] = "model"
setup["filter_name"] = ALL_REGION_NAME
setup["start"] = 2015
setup["stop"] = 2016
col_stp = ColocationSetup(**setup)
col = Colocator(col_stp)
col._check_set_start_stop()
Expand Down
Loading