torv72 / torv-reports-v4

1 stars 0 forks source link

Added Month to Tooltip in Trendline and Om Testing #54

Open torv72 opened 4 days ago

torv72 commented 4 days ago

I hope you don't mind, but I modified the code in both the om_testing and the trend line_figure .Rmd files so that when you hovered over a point on the graph it will now show the month and year the sample was take instead of just the year. I had created a HTML report for my client at The Valley Club and he gave me this feedback. He samples twice a year and found it useful if I would include the month. So I did.

I modified line 104 in the trend line_figure to the following:

  tooltip = paste0("<b>", sprintf("%.2f", avg_measurement_result, 2), unit, "</b><br><span style='font-size:12pt;'>", format(plot_date, "%B %Y"), " (", n, ")</span>")
)

I modified line 96 in the om_testing to the following:

  tooltip = paste0("<b>", sprintf("%.2f", avg_measurement_result, 2), "%</b><br><span style='font-size:12pt;'>", format(plot_date, "%B %Y"), " (", n, ")</span>")
)

Originally I tried to include the actual day but that was getting more involved than the simple changes I made. I think there is some additional work that would have to be done in order to make that work.

z3tt commented 3 days ago

Great work! I only had yearly samples in the examples I've run but it totally makes sense to add it, especially in case there are multiple sampling dates per year. I'll also update the respective code for the OM trendline plots (they have their own script because of the depth

Would you like to have the day as well? I personally think it may be a bit too much information but if it's helpful I'll add it.

Only adjustment I suggest for the tooltips is another linebreak as the labels get pretty long now. Screenshot 2024-10-21 at 11 01 54 Screenshot 2024-10-21 at 11 07 14

torv72 commented 3 days ago

I like your idea of adding another line as you illustrated. I’ll double check but I’m pretty sure that the “fix” I did in the trend line plots also worked on the water as well.  The OM plots had to be done separately though. 

To test water try: The Briarwood, 2024-05-09, zip code = 59101. Will be at my computer in a little bit. 

torv72 commented 2 days ago

I agree with you on the day.  Month works nearly 95% of the time.   There may be scenarios where it would be nice/helpful to have.  Ugh….lets include if it is easy enough to do. If not then let’s hold off.

z3tt commented 2 days ago

No problem. Screenshot 2024-10-21 at 15 06 42

torv72 commented 2 days ago

That works!

torv72 commented 2 days ago

Looks like the tool tip is always showing the same January 2, 2024 date. It is not showing the correct date.

Here is a screenshot from The Club at Flying Horse North Course. It should be showing October 17, 2024. Instead it is showing the January date.

Screenshot 2024-10-21 at 7 59 43 AM
z3tt commented 2 days ago

I can't code as fast as you find things I am currently fixing  😆 On it, but also working on something else and don't want your code to break.

It might not be worth for the current round of changes, but next time we need to make sure that you can work on a main branch while I am developing things.

torv72 commented 2 days ago

No worries! I’m working on some other reports and will be at my computer for most of the day.

On Oct 21, 2024, at 8:12 AM, Cédric Scherer @.***> wrote:

I can't code as fast as you find things I am currently fixing 😆 On it, but also working on something else and don't want your code to break.

— Reply to this email directly, view it on GitHub https://github.com/torv72/torv-reports-v4/issues/54#issuecomment-2426804919, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVVJE25ODGB7UJERUZ24ZOTZ4UDTVAVCNFSM6AAAAABQHZGDLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRWHAYDIOJRHE. You are receiving this because you modified the open/close state.

z3tt commented 2 days ago

Just realized that showing month makes no sense if we have yearly averages. Going to add a logic that it's only used when season is not set to "all" or how do we solve this?

torv72 commented 2 days ago

Unless I’m misunderstanding I don’t think it matters. It’s more of a reference for the client as far as when the samples were taken. At least on the soil tests that was what the one client had commented on. I see what you mean for the OM charts though. Makes sense.

On Oct 21, 2024, at 8:23 AM, Cédric Scherer @.***> wrote:

Just realized that showing month makes no sense if we have yearly averages. Going to add a logic that it's only used when season is not set to "all" or how do we solve this?

— Reply to this email directly, view it on GitHub https://github.com/torv72/torv-reports-v4/issues/54#issuecomment-2426836175, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVVJE23NLOS4I3UHIDG7JY3Z4UE6ZAVCNFSM6AAAAABQHZGDLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRWHAZTMMJXGU. You are receiving this because you modified the open/close state.

z3tt commented 2 days ago

It matters as either (a) the displayed date doesn't make sense if the data is aggregated across multiple sampling dates (when picking one of the available sampling dates per year) or (b) the averages are not aggregated by year anymore as it split for each date to show the correct tooltips (when using the exact date).

torv72 commented 2 days ago

I see what you mean now. It was easy to do with The Valley Club Irwin as they only have a few testing dates in comparison to the likes of Maroon Creek Club. Maroon Creek Club because of the history going so far back, it makes sense to only show the year.

So if I am understanding you correctly, we revert back to year only then?

z3tt commented 2 days ago

For now it's solved to show detailed date information in case there is only 1 sampling date per year but year only otherwise. We could think about adding all dates that have been sampled for each year but not sure if this is valuable or too much?

torv72 commented 2 days ago

Sounds good. It's hard to say how valuable it would be to have all dates. It just came up with the client from The Valley Club who was finding it valuable to be reminded when the samples were taken.

z3tt commented 2 days ago

In that case, are there multiple sampling dates per year or always just one? Just curious if we are able to make that client happy no natter if it's not shown in all other reports :)

torv72 commented 2 days ago

In 9 out 10 cases, the most I will take samples from one property is twice per year. Once in the spring and again in fall. Otherwise it is once per year.

The client is The Valley Club Irwin and The Valley Club Fazio.

I see what is happening now. When I look at Trends Over Time it is averaged by month and not by year (maybe when there is less than 1 year of data?????) That’s where the confusion is lying. Look at the Trends Over Time GREEN.

Where as you look at Maroon Creek Club under the same Trends Over Time GREEN and you see this. It’s by year all by year even 2024 and I’ve been there twice this year.

Is there a point in the code where once there is enough sampling data it goes from month to year???? Hopefully I’m explaining this OK.

On Oct 21, 2024, at 12:48 PM, Cédric Scherer @.***> wrote:

In that case, are there multiple sampling dates per year or always just one? Just curious if we are able to make that client happy no natter if it's not shown in all other reports :)

— Reply to this email directly, view it on GitHub https://github.com/torv72/torv-reports-v4/issues/54#issuecomment-2427468365, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVVJE2724M7P64QUAELLJA3Z4VD7XAVCNFSM6AAAAABQHZGDLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRXGQ3DQMZWGU. You are receiving this because you modified the open/close state.

torv72 commented 2 days ago

I didn't know if you saw my comment so I reopened it. The the following is the report function for The Valley Club Irwin.

generate_report(.site_name = "The Valley Club Irwin ", .zip_code = 83333, .date_sample_submitted = "2024-10-08", .start_date = "2017-01-01", .om_seasons = "season", .om_stats = "average", .warm_or_cool = "cool", .acid_extract = "Mehlich", .include_results_interpretation = "No", .include_sand_fraction = "No", .draw_beeswarm = "No", .output = c("pdf", "html"))

Screenshot 2024-10-21 at 1 15 42 PM

The following is the report function for Maroon Creek Club

generate_report(.site_name = "The Maroon Creek Club", .zip_code = 81611, .date_sample_submitted = "2024-09-18", .start_date = "2017-01-01", .om_seasons = "season", .om_stats = "average", .warm_or_cool = "cool", .acid_extract = "Mehlich", .include_results_interpretation = "No", .include_sand_fraction = "No", .draw_beeswarm = "No", .output = c("pdf", "html"))

Screenshot 2024-10-21 at 1 18 01 PM

As I mentioned above, is it possible that in the code if there are "x" of sample dates, it is NOT averaging by year??? Once there is enough samples dates then it begins to do so like Maroon Creek Club???

z3tt commented 2 days ago

Yes, that's the logic they have set up for you back then. Not sure what's the implications for this issue though.

Do you mean we should use the label in line with this? But that would also mean that it shows "year" as a label in case there is only one sampling date (but more than 2+ years of sampling data).

Or are you questioning the whole logic here?

torv72 commented 1 day ago

I was just trying to confirm if I understand the logic. To be honest I’m not sure I’m 100% on it but I will get there.

On Oct 22, 2024, at 1:49 AM, Cédric Scherer @.***> wrote:

Yes, that's the logic they have set up for you back then. Not sure what you're pointing at though. Do you mean we should use the label in line with this? But that would also mean that it shows "year" as a label in case there is only one sampling date (but more than 2+ years of sampling data).

— Reply to this email directly, view it on GitHub https://github.com/torv72/torv-reports-v4/issues/54#issuecomment-2428514268, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVVJE27VNBCWVGO4NWZHS73Z4X7R5AVCNFSM6AAAAABQHZGDLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRYGUYTIMRWHA. You are receiving this because you modified the open/close state.

z3tt commented 1 day ago

The logic is based on year_range[2] - year_range[1] < 2, i.e. if the difference between the minimum and maximum year is less than two it uses monthly information and yearly otherwise.

torv72 commented 21 hours ago

Let’s roll with how it makes sense. Other than The Valley Club, no one has made mention of it. For him, it was just a reference/reminder when did these samples get taken last.

I like how the OM report is set up currently. I use Maroon Creek as an example. Most of the time it will be seasonal and once a year sampling. I find seeing the sampling dates on past years helpful.

The nutrients wil vary between once a year and twice a year sampling depending on client. Again Maroon Creek Club makes sense to me. I see year only even though I was there twice this year. Makes perfect sense and I don’t need to know when I was there. The Valley Club is relatively new and doesn’t have the history. Once it does, switching to an aggregate average for the year and showing only the year makes sense.

So with all of that said and hopefully understanding what you are saying, we are set up correctly then? If so, let’s close it. If not tell me I am a “slow learner” :)- and let’s figure it out.

On Oct 23, 2024, at 3:03 AM, Cédric Scherer @.***> wrote:

The logic is based on year_range[2] - year_range[1] < 2, i.e. if the difference between the minimum and maximum year is less than two it uses monthly information and yearly otherwise.

— Reply to this email directly, view it on GitHub https://github.com/torv72/torv-reports-v4/issues/54#issuecomment-2431386474, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVVJE24DLHRCZAJ2TNAQAPLZ45Q5FAVCNFSM6AAAAABQHZGDLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZRGM4DMNBXGQ. You are receiving this because you modified the open/close state.