-
Notifications
You must be signed in to change notification settings - Fork 35
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
Use rts_gmlc data package for Double Loop #183
Use rts_gmlc data package for Double Loop #183
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #183 +/- ##
=======================================
Coverage 89.49% 89.49%
=======================================
Files 67 67
Lines 8238 8238
=======================================
Hits 7373 7373
Misses 865 865 ☔ View full report in Codecov by Sentry. |
@@ -32,7 +32,7 @@ | |||
from pyomo.common.fileutils import this_file_dir | |||
import pandas as pd | |||
from pathlib import Path | |||
from dispatches_sample_data import rts_gmlc | |||
from dispatches_data.api import path |
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.
Is this path related to pathlib's Path? The names seem a bit too similar
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.
It is related in the sense that it's a function that returns a pathlib.Path
object: https://dispatches-data-packages.readthedocs.io/en/latest/#dispatches_data.api.path.
As for whether the name is too similar: I think in general it's a fair point. I think I chose path()
as opposed to e.g. get_path()
to more closely resemble the importlib.resources
API, where the "getters" are also named without the get_
prefix.
I'm open to revisit this (or add an alias, to preserve compatibility?), but this would have to be done in the gmlc-dispatches/data-packages repository first.
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.
I'm okay with it
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 good to me. Thank you @lbianchi-lbl for updating the notebooks!
Summary/Motivation:
rts_gmlc
data package throughdata_packages.api
to access RTS-GMLC data for Double Loop simulations for consistency with the other data packages.Changes proposed in this PR:
dispatches-data-packages
as a requirementdispatches-rts-gmlc-data
(providing therts_gmlc
data package) as a requirementLegal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: