-
Notifications
You must be signed in to change notification settings - Fork 910
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
Changed US-MIDW-MISO parser to use EIA instead #3009
Conversation
Thanks for your contribution! ⚡️ A few things to be aware of:
Potential reviewer(s): @systemcatch |
Hmm. I don't have a strong opinion here. I'm a bit hesitant to delete it if people are finding it useful. It also makes it slightly easier to switch back if that becomes relevant. On the other hand, it could clutter the parser folder a bit if we keep them around. |
I would prefer the old parser is left in place, the file size is tiny and it would save quite a bit of time if you wanted to switch back in the future. How about marking it with a comment linked to relevent issues and moving it to an |
I'm usually all for deleting code that we already have in the git history. But in this case, I think it makes sense to keep.
I might be overthinking this, but I see an We could have a script that prints out parsers that are not used – we could even do a unit test that prints out parsers that are not used. 🤔 |
That's a good idea!
…On Wed, Feb 24, 2021 at 1:05 PM Kenneth Skovhus ***@***.***> wrote:
I'm usually all for deleting code that we already have in the git history.
But in this case, I think it makes sense to keep.
How about marking it with a comment linked to relevent issues and moving
it to an old_parsers directory?
I might be overthinking this, but I see an old_parsers folder might
quickly become out of sync as we would start using those by changing the
zones.json file.
We could have a script that prints out parsers that are not used – we
could even do a unit test that prints out parsers that are not used. 🤔
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3009 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMUIKE4Y2OEISI5WD4GEOLTATTR3ANCNFSM4YEJL7QQ>
.
--
Olivier Corradi
Founder, CEO
<https://www.linkedin.com/in/oliviercorradi> <https://twitter.com/corradio>
tmrow.com
|
👍 I agree. In that case, are there any changes I should do to this PR? |
LGTM |
* Changed US-MIDW-MISO parser to use EIA instead * Added delay to zone config * Updatd DATA_SOURCES
Resolves #2725
The current data source is coming in very spotty. We investigated the opportunity of estimating the unknown breakdown, but it turned out to be rather difficult, and we don't want to make too many assumptions.
It's a bit unfortunate because EIA isn't real-time (It's delayed up to 30 hours afaik), so it won't always show up on the map. It would be shown when the historical view has been implemented #2393
I backed up the old data, so we can potentially switch back if we find a fix.