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

Replace deprecated applymap() and use two decimal places for formatting #1743

Merged
merged 3 commits into from
Apr 11, 2024

Conversation

caleywoods
Copy link

Description

This attempts to resolve #1642

Additional information

Include any of the following (as necessary):

  • For a project view (/projects/{pk_id}), we were previously formatting the total hours in the data table using :.0f, I've updated that to format using :.2f instead. This seems to correctly stop truncating hours and the test suite passes.

  • The original ticket also mentions DataTable.applymap() being deprecated and replaced by DataTable.map(). I found that DataTable.map() was already being used in the utilization/analytics.py file but the terminal complained about DataTable.applymap() in utilization/views.py on line 187, I've replaced DataTable.applymap() with DataTable.map() and that didn't seem to upset anything.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.17%. Comparing base (bae914e) to head (796a23a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1743   +/-   ##
=======================================
  Coverage   94.17%   94.17%           
=======================================
  Files          66       66           
  Lines        4170     4170           
=======================================
  Hits         3927     3927           
  Misses        243      243           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -184,7 +184,7 @@ def get_context_data(self, **kwargs):
"headcount_data": headcount_data_frame.pivot(
index="start_date", values="headcount", columns="organization"
)
.applymap("{:.0f}".format)
.map("{:.0f}".format)
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be :.2 for consistency as well?

Copy link
Author

Choose a reason for hiding this comment

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

I originally made a commit to change it to :.2f but it seems like it's used to display the headcount within the data table on the route utilization/analytics, I may have assumed incorrectly that head count was going to be a whole number. That's an easy enough change if we'd like it to be that way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this one is a whole number and so 0 decimal places is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks y'all! You are right, 0 decimal places is correct here. Merge at will!

Copy link
Contributor

@neilmb neilmb left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for finding and fixing this @caleywoods

@@ -184,7 +184,7 @@ def get_context_data(self, **kwargs):
"headcount_data": headcount_data_frame.pivot(
index="start_date", values="headcount", columns="organization"
)
.applymap("{:.0f}".format)
.map("{:.0f}".format)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this one is a whole number and so 0 decimal places is correct.

@caleywoods caleywoods merged commit c6b7bc9 into main Apr 11, 2024
2 checks passed
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.

Project data table shows truncated hours
4 participants