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

Fix incorrect x axis date labels #171

Merged
merged 1 commit into from
Jan 26, 2025
Merged

Fix incorrect x axis date labels #171

merged 1 commit into from
Jan 26, 2025

Conversation

eldcn
Copy link
Contributor

@eldcn eldcn commented Jan 18, 2025

Fix incorrect x axis date labels

♻️ Current situation & Problem

  • We have had several iterations on fixing the correct date formatting for x axis in health charts, e.g. (Chart scaling #78 and Investigate: Chart XValues too precise #92). This due to how the charts library expects the values. We display the dates in the x axis and must supply the corresponding double value to the chart. We have tried in the last iterations to transform the values to reduce the distance between values in order to also reduce the spreading of the graph.
  • In the last iteration we were mapping a Date to a double of the pattern YYYY,DD where DD was calculated as day of year / 365 rounded to max 2 decimal places and were mapping it back to a date format the label. Chart library on the other hand, was expecting the decimal places to be rounded to max 2 places. Rounding was causing the issue and we were not able to derive the initial date correctly

Adapted the approach to:

  1. Given a list of measurements and their corresponding dates, pick the oldest date
  2. Map the dates as follows for the selected time range:
    • Daily: Diff in days from the oldest date
    • Weekly: Diff in weeks from the oldest date
    • Monthly: Diff in months from the oldest date
  3. The diff is used ultimately as the x value, e.g. for daily time range:
2025-01-01 → 0
2025-01-04 → 3
2025-01-06 → 5
2025-01-13 → 12
2025-02-14 → 44
  1. The library supplies us back the value and expects a formatted label text for it, e.g. 5 -> we derive the initial date as 2025-01-06 and ultimately format it as Jan 06
  2. Unit tests added to cover all three time ranges
  3. Recording:
chart_x_labels.mp4

⚙️ Release Notes

Add a bullet point list summary of the feature and possible migration guides if this is a breaking change so this section can be added to the release notes.
Include code snippets that provide examples of the feature implemented or links to the documentation if it appends or changes the public interface.

📚 Documentation

Please ensure that you properly document any additions in conformance to Spezi Documentation Guide.
You can use this section to describe your solution, but we encourage contributors to document your reasoning and changes using in-line documentation.

✅ Testing

Please ensure that the PR meets the testing requirements set by CodeCov and that new functionality is appropriately tested.
This section describes important information about the tests and why some elements might not be testable.

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@eldcn eldcn linked an issue Jan 18, 2025 that may be closed by this pull request
1 task
Copy link

codecov bot commented Jan 18, 2025

Codecov Report

Attention: Patch coverage is 57.40741% with 23 lines in your changes missing coverage. Please review.

Project coverage is 40.72%. Comparing base (b98ea73) to head (d835d6a).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ord/bdh/engagehf/health/symptoms/SymptomsScreen.kt 0.00% 17 Missing ⚠️
...tanford/bdh/engagehf/health/HealthUiStateMapper.kt 85.30% 5 Missing ⚠️
...ford/bdh/engagehf/health/components/HealthChart.kt 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #171      +/-   ##
============================================
+ Coverage     40.54%   40.72%   +0.18%     
- Complexity      858      866       +8     
============================================
  Files           299      299              
  Lines         11421    11434      +13     
  Branches       1716     1723       +7     
============================================
+ Hits           4630     4655      +25     
+ Misses         6326     6319       -7     
+ Partials        465      460       -5     
Flag Coverage Δ
uitests 36.01% <ø> (+0.03%) ⬆️
unittests 32.10% <57.41%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../engagehf/health/symptoms/SymptomsUiStateMapper.kt 94.90% <100.00%> (ø)
.../bdh/engagehf/health/symptoms/SymptomsViewModel.kt 94.26% <100.00%> (ø)
...ford/bdh/engagehf/health/components/HealthChart.kt 0.00% <0.00%> (ø)
...tanford/bdh/engagehf/health/HealthUiStateMapper.kt 68.58% <85.30%> (+10.88%) ⬆️
...ord/bdh/engagehf/health/symptoms/SymptomsScreen.kt 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b98ea73...d835d6a. Read the comment docs.

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Really like the new visualizations @eldcn; thank you for the additional tests and getting this in place! 🚀

@PSchmiedmayer PSchmiedmayer added bug An unexpected problem enhancement New feature or request ENGAGE HF ENGAGE-HF-specific issues labels Jan 19, 2025
eldcn added a commit that referenced this pull request Jan 26, 2025
# *Symptom score categories drop down items and date labels in charts*

## ♻️ Current situation & Problem
- Fixes #161 - invisible titles of categories
- Noticed additionally similar issue as #171 and adapted for the symptom
scores chart also



![symptom_scores_dropdown](https://github.com/user-attachments/assets/0359cc21-9d24-4ce0-b34b-d7b340745727)


## ⚙️ Release Notes 
*Add a bullet point list summary of the feature and possible migration
guides if this is a breaking change so this section can be added to the
release notes.*
*Include code snippets that provide examples of the feature implemented
or links to the documentation if it appends or changes the public
interface.*


## 📚 Documentation
*Please ensure that you properly document any additions in conformance
to [Spezi Documentation
Guide](https://github.com/StanfordSpezi/.github/blob/main/DOCUMENTATIONGUIDE.md).*
*You can use this section to describe your solution, but we encourage
contributors to document your reasoning and changes using in-line
documentation.*


## ✅ Testing
*Please ensure that the PR meets the testing requirements set by CodeCov
and that new functionality is appropriately tested.*
*This section describes important information about the tests and why
some elements might not be testable.*


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [ ] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).

---------

Co-authored-by: Kilian Schneider <[email protected]>
@eldcn eldcn merged commit 0e0d806 into main Jan 26, 2025
11 checks passed
@eldcn eldcn deleted the task/issue-169 branch January 26, 2025 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem ENGAGE HF ENGAGE-HF-specific issues enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

The axes on the vitals graphs are incorrect
3 participants