Skip to content
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

Add second graph displaying Average User Level Rating by Bite ID #1

Merged
merged 8 commits into from Jun 27, 2022

Conversation

kirk5davis
Copy link
Contributor

In your blog article - How to make a nice graph using Django and Chart.js (which I enjoyed) - you had a call to action to fork your repo and add a second graph. This PR should do the job!

Additional PR info:

  • The input csv wasn't included in the repo, so I took the opportunity to create another Django management command to load fake Bite stats/data into the model and can be invoked with python manage.py generate_fake_stats [-n num | -d del]. This command accepts a number -n or --num arg representing the number of fake records to create. There is also a delete flag -d or --del to delete all of the records prior to loading the fake records.
  • Added some clarifying HTML, adjusted the graphs, and used Pico.css for some additional styling.

@bbelderbos
Copy link
Owner

@kirk5davis awesome man, thanks! I will review later and merge it in, updating the article with a reference / mention. Appreciate it.

@bbelderbos
Copy link
Owner

Man awesome, runs perfectly and looks awesome, thanks!
I will update the article with this ...

image

Copy link
Owner

@bbelderbos bbelderbos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome


from .models import BiteStat

class BiteStatFactory(DjangoModelFactory):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a nice follow up article, I am not using this enough in Django :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would make a great follow-up article! The factory_boy package was perfect for this and I don't use it enough either, seems to be intended mostly for testing use-cases too. Under the hood it's using the awesome Faker package, and there's so many different providers for generating random values for Django Model Fields - it's really cool!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah if you want to write that up let's do it! Thanks

BiteStat.objects.all().delete()

self.stdout.write(f"Creating {number_of_records} new fake records...")
for record in range(number_of_records):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we use record? if not, I would use _ throwaway variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. We don't use record, I think I just used it to be more verbose. I'll keep the _ throwaway in mind in future code.

for row in stats:
yymm = row.completed.strftime("%Y-%m")
data[yymm] += 1
data_completion_per_month[yymm] += 1
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, better variable name

@bbelderbos bbelderbos merged commit af956d1 into bbelderbos:main Jun 27, 2022
@kirk5davis
Copy link
Contributor Author

Thanks for all the feedback on the PR! It's so cool to see it become a part of the article too! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants