Skip to content

Retrieve properly the activity for exps linked to multiple ids #257

Merged
mattiarighi merged 2 commits into
developmentfrom
development_multiple_activity_ids
Oct 1, 2019
Merged

Retrieve properly the activity for exps linked to multiple ids #257
mattiarighi merged 2 commits into
developmentfrom
development_multiple_activity_ids

Conversation

@sloosvel

Copy link
Copy Markdown
Contributor

Fixes #248

@sloosvel sloosvel requested review from jvegreg and zklaus September 18, 2019 10:09
@mattiarighi mattiarighi added the preprocessor Related to the preprocessor label Sep 18, 2019

@jvegreg jvegreg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There was a problem with the test, as we are now returing a string and not a list of strings. I fixed it.

It should not be a problem for the datafinder, as tags can be both

@zklaus

zklaus commented Sep 23, 2019

Copy link
Copy Markdown

I couldn't test yet, but just to clarify, @jvegasbsc: There are two ways lists could enter here:

  • One experiment has two activities associated. In this case this PR correctly uses only the first one.
  • One dataset spans two experiments, given as a list in {dataset: ..., exp: [historical, ssp585]}. In this case there will also be two activities, one for every experiment. Is this second case considered?

@sloosvel

Copy link
Copy Markdown
Contributor Author
* One dataset spans two experiments, given as a list in `{dataset: ..., exp: [historical, ssp585]}`. In this case there will also be two activities, one for every experiment. Is this second case considered?

Yes, this is also considered. If the experiment is a list, the code loops over experiments. And for each experiment, if it is associated to multiple activities it only considers the first one.

The test needed to be fixed because if there is only one experiment, what gets returned is just the value of the activity, instead of list with one value. But if there are multiple exps, the activity is returned as a list as well. At least that's what I get when I try it.

@mattiarighi

Copy link
Copy Markdown
Contributor

@axel-lauer does this solve the problem you mentioned last week?

@zklaus zklaus left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Works nicely!

@mattiarighi mattiarighi merged commit 5a042d0 into development Oct 1, 2019
@mattiarighi mattiarighi deleted the development_multiple_activity_ids branch October 1, 2019 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preprocessor Related to the preprocessor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug in activity_id determination for some cmip6 experiments

5 participants