Skip to content

Set FBA columns and column dtypes#362

Merged
catherinebirney merged 3 commits intorecursive-refacfrom
fba_cols
Aug 24, 2023
Merged

Set FBA columns and column dtypes#362
catherinebirney merged 3 commits intorecursive-refacfrom
fba_cols

Conversation

@catherinebirney
Copy link
Copy Markdown
Contributor

@catherinebirney catherinebirney commented Aug 24, 2023

  • FBA generation only returns columns identified as "required: True" in schema.py, can also include optional columns, such as "Suppressed". Previously, all columns in schema.py where added to FBA.
  • All empty cells are now represented by np.nan (or 0) rather than a mix of np.nan and None

@bl-young
Copy link
Copy Markdown
Contributor

bl-young commented Aug 24, 2023

were you finding issues in some cases? I'm not sure the best way to test this - looks reasonable to me.

Suppressed still shows up when needed?

@catherinebirney
Copy link
Copy Markdown
Contributor Author

catherinebirney commented Aug 24, 2023

Suppressed does show up when we want it - so Suppressed is included in the EIA_MECS_Energy FBAs. But now a column of np.nan is not added for no reason.

There wasn't a real issue with the dtypes, but the FBAs we were generating had a mix of None and np.nan columns, so I changed it for consistency.

@bl-young
Copy link
Copy Markdown
Contributor

bl-young commented Aug 24, 2023

ok, i still get confused since there is also the new flowby_config.yaml so we will have to consolidate at somepoint

@catherinebirney
Copy link
Copy Markdown
Contributor Author

catherinebirney commented Aug 24, 2023

Yes, we do need to consolidate, but right now FBAs are still generated with our original method so that is the code I tweaked

@catherinebirney catherinebirney merged commit f046bd8 into recursive-refac Aug 24, 2023
@catherinebirney catherinebirney deleted the fba_cols branch August 24, 2023 21:14
@bl-young
Copy link
Copy Markdown
Contributor

bl-young commented Aug 25, 2023

@catherinebirney this seems to have caused some issues with collapse_fbs_sectors() because the sectors are showing up as 'nan' strings instead of np.nan (i believe)

@bl-young
Copy link
Copy Markdown
Contributor

bl-young commented Aug 25, 2023

fixed with 1c4a1e2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@bl-young bl-young bl-young approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@catherinebirney @bl-young