-
Notifications
You must be signed in to change notification settings - Fork 9
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
Data Analysis: Do high emissions predict reporting non-compliance? #147
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for radiant-cucurucho-d09bae ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I've reduced the file size of the graphs down to a cumulative 2MB. I did this by dropping some of the data displayed on hover. We could reduce the file size even more by not displaying every single observation in the scatterplots and only displaying a handful of of the buildings that have standard emissions (it's hard to tell them apart anyways). We could also just convert the images to static PNG files. Let me know what you think is best. |
@colton-lapp - I meant to comment when I pushed up my fixes - I've added some date stamps to the blog posts and reodered it so yours comes first (since it's newer). I'm fine with that file size, but it looks like there's some responsiveness issues with the graphs - if you can fix those, I'm good with it, but otherwise we could move to images. Here's an example:
Also is there a way to note dependencies for you Jupyter notebook? I tried running it locally but had to manually install dependencies like |
…s, plotly, and running regressions. Cleaned up notebook comments
@colton-lapp - wanted to check in on this, it looks like Pytest is failing, and if you need help using the graph images just on mobile, let me know! I also showed the preview to one of our partner's at Climate Reality Chicago (who asked for this research question), and they were a big fan, and said:
I agree with her feedback on adding years, and I think that would be a simple improvement to each graph, just to make sure if it gets screenshotted and shared it's really clear what data we used. Even adding Chicago in there might be helpful, but 🤷🏻♂️ |
</p> | ||
|
||
<p class="constrained"> | ||
Many buildings in the publicly available data |
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.
Let's change "the publicly available data" to "Chicago's building benchmarking data" across your article and link the first time to the main data source we use on the site.
rel="noopener noreferrer" | ||
>don't report emissions data.</a | ||
> | ||
Is there a pattern to which buildings fail to report? Anecdotally, it's |
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.
I'd change this "Anecdotally..." sentence to "Our team has noticed that some high emissions buildings stop reporting, while more efficient buildings tend to keep reporting year after year." There unfortunately has not been real press coverage of this data 😭
|
||
<p> | ||
The graph below depicts the count of buildings that did and did not | ||
report emissions data every year. |
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.
I'd change "every year" to "each year", unless it's a cumulative graph
Finally got some time to work on this and addressed everything you mentioned I think:
Final todo: Fix regression table at the bottom to be prettier |
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.
@colton-lapp - the resize change works great, but if you test your preview on mobile, the font sizes are too small to read. Can you re-export those PNGs with larger font sizes targeted to mobile? You could also move the legend below the graphs (if that's an option) and you need more room.
I've also made a few minor tweaks to the blog post to improve the styling, particularly the regression table:
} | ||
|
||
checkScreenSize(): void { | ||
this.isMobile = window.innerWidth <= 768; |
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.
Not blocking, but in the future I'd recommend doing this with CSS, which can handle checking screen size changes automatically. You'd do something like:
<!-- html -->
<img class="mobile-only" ....>
<iframe class="desktop-only">
.graph-cont {
.mobile-only { display: none; }
// Mobile styling
@media (max-width: $mobile-max-width) {
.desktop-only { display: none; }
.mobile-only { display: block; }
}
}
Description
This pull request creates:
The blog post investigates a question raised in #114 - does poor performance correlate with non-reporting? The short answer is no, I didn't find that pattern in the data.
The data analysis in the Jupter notebook consists of the following steps:
These findings are then summarized in a new blog post.
A couple other notes:
This is my first time creating a PR for a public repo and for this project specifically so happy to restructure any work or accept any feedback! I'm expecting some heavy feedback on files committed (i.e. new packages used in requirements.txt, python virtual environment, directory structure).
Fixes #114
Testing Instructions
I would recommend pulling, running docker-compose up and looking at my blog. Additionally, check out the Jupyter notebook to verify that I'm analyzing the correct variables and don't have data analysis mistakes, etc. To see the interactive html graphs in the notebook, you have to view the Jupyter file in NBViewer as it won't render in Github
Checklist: