Conversation
jaime-82
requested review from
christophertull,
dmarulli,
jin-current,
mdowell12 and
pat-current
as code owners
mdowell12
left a comment
mdowell12
left a comment
There was a problem hiding this comment.
@jaime-82 looks like the linter is failing. You can fix that by running black . from the root of the repo with your virtualenv turned on. Let me know if you have trouble. I can fix it, but I'd prefer if you did it before merging.
Otherwise, left one question and the query LGTM.
| , district_source | ||
| , meter_type | ||
| from | ||
| irrigation_detection |
There was a problem hiding this comment.
@jaime-82 how is irrigation_detection created? Will this fail if someone hasn't manually created it already?
If so, could you add a create table statement somewhere in the sql folder so that people know how to create the table?
There was a problem hiding this comment.
Just adding in - this is a table that will need to be created by Ryan Hildebrant as part of deploying the irrigation detection model, which should happen in the next week or two. Alternately, I don't know if a table with a different name already exists?
There was a problem hiding this comment.
That's all good and I'm not worried about a conflicting table. I was just hoping someone could add the create table statement to this repo's ./sql folder so that a future open source user could easily set it up.
|
Hey @jaime-82 checking in on this - are you still hoping to merge the work? |
| , district_source | ||
| , meter_type | ||
| from | ||
| irrigation_detection |
There was a problem hiding this comment.
That's all good and I'm not worried about a conflicting table. I was just hoping someone could add the create table statement to this repo's ./sql folder so that a future open source user could easily set it up.
|
Looks like the linter is failing. Could someone 1) rebase this off the latest code and 2) run |
|
I'm creating a new clean PR |
|
this is the new clean PR #196 |
3 participants
No description provided.