Add plots of editors load in current-reviews-data.ipynb #24
Conversation
|
hey @santisoler is this PR still a WIP or is it meant to be reviewed? many thanks!! 🙌🏻 |
|
Hi @lwasser! Sorry for not updating the status of this PR. The plots are working now, but maybe we want to improve how they look or customize them a little bit. I would appreciate if you could review them. Thanks for asking! |
|
hi @santisoler wonderful !! thank you. i'll put this on my list of pr's to review!! thank you again for the help here!! |
There was a problem hiding this comment.
@santisoler this is looking really nice! thank you!
i left a few suggested changes to the code. then i have a few followup requests
- can you please make the plot color purple
color="purple"we use that color on other plots in our metrics dashboard! - Can you please only list the y axis label once rather than on each plot? if this is hard let's modify the y axis label to be "reviews per quarter"
- this is just a question - how hard would it be to move the title - which is the editor name - to the left or right side of the plot? if that is super hard, let's skip it.
rOpenSci's plot looks like this:
what i dislike about the plot is i think there needs to be more space between each facet. but what i like is that each row has a label with the editor name and the y axis is only labeled once.
If you dont' have time to play with the plot axis' too much id' say just change the colors, and clean up the filtering of the data and the vmin date
then let's open another issue to clean up the labels .
thank you again for this!
| "source": [ | ||
| "charts = []\n", | ||
| "for editor in editors:\n", | ||
| " if editor in ignore_editors:\n", |
There was a problem hiding this comment.
can you please in a higher up cell filter out
ignore_editors so it's not mixed into this plot cell? that will just make things a bit cleaner.
that could look like this:
# Remove TBD and editors that are no longer active
current_editors = [editor for editor in editors if editor not in ignore_editors]There was a problem hiding this comment.
then you can just loop through current editors
for editor in current_editors:
| "# Get axes min and max values so all plots have the same scale\n", | ||
| "\n", | ||
| "# Get vmin and vmax for the dates\n", | ||
| "min_date = reviews_df.created_at.min().to_period(\"Q\").start_time\n", |
There was a problem hiding this comment.
Let's filter only on the currently open reviews and reviews with editors that are not in the ignore list.
ignore_editors = ["TBD", "lwasser", "xmnlab"]
current_reviews = edits[~edits['editor'].isin(ignore_editors)]
min_date = current_reviews.Date.min().to_period("Q").start_timeNOTE: let's add lwasser and xmnlab to the ignore list for these plots!
then we can grab the min time from the list of current reviews! and the plots will look nicer !
| "# Get vmin and vmax for the dates\n", | ||
| "min_date = reviews_df.created_at.min().to_period(\"Q\").start_time\n", | ||
| "end_of_this_quarter = pd.Timestamp.today().to_period(\"Q\").end_time\n", | ||
| "time_domain = (min_date, end_of_this_quarter)\n", |
There was a problem hiding this comment.
Right now the ticks have a .5 increment. can we please make that increment 1 instead?
|
@santisoler here is another idea if you are short on time! We could merge this PR as is and if you have time in the future - you could work on edits. If not, no worries. I'm suggesting this only because i am going to reorganize this repo a bit to make space for other types of organization metrics. I'd love to get this pr merged without rushing you! The plot works and is great as is. We can finesse it in future pr's. Does that work for you? |
Improve the style of the plots and hide input cells by adding the `"remove-input"` tag to their metadata.
* feat: add new review plots * fix: reorganize content * fix: function altair
|
Hi there 👋🏻 I've created a small package in metrics-theme/src called metrics theme. The functions.py module can be added there, and I added a small plot theme module as well, so our plots are all consistent! |
|
Hi @lwasser! Sorry for the delayed reply! Thanks for merging the PR. I started working on the points you mentioned, but haven't pushed yet (coming back from vacations requires some time to catch up). I'll open another PR with those changes if that works for you. |
Add a new "Editors load" section to the
current-reviews-data.ipynbwith plots of number of contributions by each editor in each quarter. Include a new function infunctions.pyto count number of contributions per quarter.This PR was created during SciPy2024 sprints.