NYCPlanning / data-engineering-qaqc

streamlit app for data engineering
https://edm-data-engineering.nycplanningdigital.com
1 stars 0 forks source link

Control version from page file #204

Closed SashaWeinstein closed 2 years ago

SashaWeinstein commented 2 years ago

Small PR, only needs one reviewer 📆 I am curious about your thoughts Andrew on this refactor. I think that a sensible convention here is that sidebar content and any user input generally should come from the home page file if possible.

SashaWeinstein commented 2 years ago

I tested the code by manually setting usetype_changes to None above the if/else block

abrieff commented 2 years ago

I don't think you can make the user input convention more broadly, since there are report specific user inputs. (take the # of use types to display for these graphs, for example.) I'm not sure i'd even make it a goal - if something is component specific, I think you'd rather the whole component be self-contained. There's going to be enough exceptions to the rule to make the rule itself kind of moot otherwise.

For the sidebar, I think that makes sense though.