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

Support aux data in from_matched() #248

Merged
merged 27 commits into from
Aug 28, 2023
Merged

Conversation

jsmariegaard
Copy link
Member

@jsmariegaard jsmariegaard commented Aug 25, 2023

We would like to support aux data upon creating a Comparer with the from_matched() method.

A number of things have been changed in this PR. The most important are:

  • from_matched() now has a aux_items argument
  • from_matched() calls the Comparer class method from_matched_data() instead of creating
  • from_matched() can now also take a dfs0 or a mikeio.Dataset in which case the quantity will be taken from the ItemInfo of the observation
  • Comparer.sel() method now also filteres raw_data in time as it was very confusing to do cmp.sel(time="2019-01").plot.timeseries() and then get a plot also outside this time range
  • Comparer now got a quantity property instead of an attribute avoid potential mis-alignment with info in Dataset
  • Comparer variable_name renamed to quantity_name - next step is to use quantity.name instead
  • A new notebook with examples have been added
  • New tests

Things to consider:

  • What should the default aux_items be? None? All in the input which is neither obs nor model?
  • x, y columns are not yet supported in from_matched() but they could
  • What default name should we give the Comparer? Or should we create a name argument in from_matched()

@jsmariegaard jsmariegaard marked this pull request as ready for review August 27, 2023 19:32
@jsmariegaard jsmariegaard merged commit 3af534b into main Aug 28, 2023
@jsmariegaard jsmariegaard deleted the aux-data-in-from-compared branch August 28, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants