Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Transform eia861 short form #3565
Transform eia861 short form #3565
Changes from 9 commits
e37f59e
2ac0190
6ce3705
9f84516
574dbbc
a33045a
dde8ccd
40831bc
6a2e5fc
ebda8ca
8fa1ac9
f9011bd
6992a23
da5606c
4a8ab46
0123d39
9c07b29
6ec2e2e
1e2427c
fe20029
ec145d1
39428e9
d555a80
55acf8e
3972cd4
cf51b0e
1f00d77
289aaae
e444626
91a3571
db8b823
3aefd2e
5a522ca
c1f79be
11dea5d
8068520
6153600
c89149d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 state definitely a primary key?
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.
'state' is not the primary key independently. It is one of the variables that contribute to the compound primary key. All the fields stated under the 'primary_key' all come together to form the compound primary key. Am I wrong about this?
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.
You're right, I just wanted to check! (I looked at it in a notebook and it seems like
balancing_authority_code_eia
doesn't need to be part of the primary key (the tables are the same length if you drop duplicates withoutbalancing_authority_code_eia
) however, it's possible that in the future this column could be part of the primary key? @cmgosnell what do you think here?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.
Maybe this would have consequences for other tables and so we don't want to deal with it right now, but I think this should probably be
num_customers
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.
In the src.pudl.metadata.fields, there's the customers key in the JSON fields. There's no num_customers or total_customers. Please check to confirm that this is correct so I'll know if I still need to change this.
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.
What do you mean by "customers key"?
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.
@Nancy9ice what I meant here is that I think we originally did a poor job of naming this column, or that it was named before we had thought about our naming conventions, so I was suggesting that we take this opportunity to rename
customers
which is pretty vague, to something that provides a bit more context about the column, likecustomers_num
ornum_customers
though searching throughfields.py
for other instances of_num
andnum_
it doesn't seem as if we have established a strong convention right now, so maybe we should just ignore this for now and deal with it in a future round of renaming / deeper EIA-861 integration.