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 1 commit
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
51 changes: 30 additions & 21 deletions pyaerocom/colocation/colocation_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,36 @@ def validate_no_forbidden_keys(self):
for key in self.FORBIDDEN_KEYS:
if key in self.model_fields:
raise ValidationError
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

# @model_validator(mode="after")
Copy link
Member

Choose a reason for hiding this comment

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

Can we not just leave this in?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can. But then we have to remove the return self above. I'm not sure if this might lead to other strange behavior

# @classmethod
# def validate_obs_config(cls, v: PyaroConfig):
# if v is not None and cls.obs.config.name != cls.obs_id:
# logger.info(
# f"Data ID in Pyaro config {v.name} does not match obs_id {cls.obs_id}. Setting Pyaro config to None!"
# )
# v = None
# if v is not None:
# if isinstance(v, dict):
# logger.info("Obs config was given as dict. Will try to convert to PyaroConfig")
# v = PyaroConfig(**v)
# if v.name != cls.obs_id:
# logger.info(
# f"Data ID in Pyaro config {v.name} does not match obs_id {cls.obs_id}. Setting Obs ID to match Pyaro Config!"
# )
# cls.obs_id = v.name
# if cls.obs_id is None:
# cls.obs_id = v.name
# return v

@cached_property
def basedir_logfiles(self):
Expand All @@ -471,27 +501,6 @@ def basedir_logfiles(self):
p.mkdir(parents=True, exist_ok=True)
return str(p)

@model_validator(mode="after")
@classmethod
def validate_obs_config(cls, v: PyaroConfig):
if v is not None and cls.obs.config.name != cls.obs_id:
logger.info(
f"Data ID in Pyaro config {v.name} does not match obs_id {cls.obs_id}. Setting Pyaro config to None!"
)
v = None
if v is not None:
if isinstance(v, dict):
logger.info("Obs config was given as dict. Will try to convert to PyaroConfig")
v = PyaroConfig(**v)
if v.name != cls.obs_id:
logger.info(
f"Data ID in Pyaro config {v.name} does not match obs_id {cls.obs_id}. Setting Obs ID to match Pyaro Config!"
)
cls.obs_id = v.name
if cls.obs_id is None:
cls.obs_id = v.name
return v

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
Loading