Add timeseries page to dashboard

[stefsmeets]

This PR adds a page for timeseries to the dashboard.

Closes #568

Todo

• Add caching
• What is the file type/format for data uploads?
• Update lime interface

[stefsmeets]

Hi @geek-yang @cwmeijer , is either of you interested in reviewing this PR?

[geek-yang]

I went over the changes roughly and most of them look pretty nice to me (ps: don't have much experience with streamlit. But I think the code itself looks pretty and very well-organized. It is very readable. I could follow the code and figure it out what are most of the functions used for. Nice lesson learned for me about streamlit 😄 ).

Just some small comments to address, before merging it to main branch. But I can give you a go already.

I put more time on testing the dashboard, especially for timeseries panel. It looks pretty awesome. And the added cache functionality also works for me perfectly. Thanks a lot for the fabulous work 👍!

[geek-yang]

I got a small concern about sonarcloud check. If we merge it as what it is for now, the coverage will drop dramatically. Since we don't want to add unit tests for each function in streamlit (also it makes no sense to do so at this point). Shall we just skip dashboard folder only for sonarcloud check?

[geek-yang]

See this doc: https://docs.sonarcloud.io/advanced-setup/analysis-scope/
Under the section "File exclusion and inclusion", it seems we can exclude files by tweaking the "analysis scope".

[elboyran]

[heart] Elena Ranguelova reacted to your message:

________________________________ From: Yang ***@***.***> Sent: Thursday, June 1, 2023 2:25:17 PM To: dianna-ai/dianna ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [dianna-ai/dianna] Add timeseries page to dashboard (PR #575) @geek-yang approved this pull request. I went over the changes roughly and everything looks pretty nice to me (ps: don't have much experience with streamlit. But I think the code itself looks pretty and very well-organized. It is very readable. I could follow the code and figure it out what are most of the functions used for. Nice lesson learned for me about streamlit👍). I put more time on testing the dashboard, especially for timeseries panel. It looks pretty awesome. And the added cache functionality also works for me perfectly. Thanks a lot for the fabulous work 👍!

________________________________ In dianna/methods/lime_timeseries.py<#575 (comment)>:

@@ -117,15 +116,15 @@ def explain(

) # extract scores from lime explainer saliency = [] - for i, _ in enumerate(labels): - local_exp = sorted(exp.local_exp[i]) + for i, label in enumerate(labels): + local_exp = sorted(exp.local_exp[label]) Well spot! Thanks for fixing it.

________________________________ In dianna/methods/lime_timeseries.py<#575 (comment)>:

# shape of local_exp [(index, saliency)]

selected_saliency = [i[1] for i in local_exp] saliency.append(selected_saliency[:]) return np.concatenate(saliency).reshape(-1, sequence, n_var) - def _calculate_distance(self, masked_data, distance_method="cosine"): + def _calculate_distance(self, masked_data, distance_method='cosine'): Just notice that your formatter prefers single quote. I use black and it turns everything to double quote. Maybe we can create an issue and put it in the developer doc about which formatter we prefer? — Reply to this email directly, view it on GitHub<#575 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAYYBWPBAZGAKNAZYTLNLR3XJCQ43ANCNFSM6AAAAAAXLDSWOM>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[stefsmeets]

I got a small concern about sonarcloud check. If we merge it as what it is for now, the coverage will drop dramatically. Since we don't want to add unit tests for each function in streamlit (also it makes no sense to do so at this point). Shall we just skip dashboard folder only for sonarcloud check?

Much of this code is still being tested, just not taken into account for coverage.

The issue is that the code is run in a subprocess, which cannot be measured automatically for coverage. This would be the way to go about it: https://coverage.readthedocs.io/en/stable/subprocess.html#configuring-python-for-sub-process-measurement

[geek-yang]

I got a small concern about sonarcloud check. If we merge it as what it is for now, the coverage will drop dramatically. Since we don't want to add unit tests for each function in streamlit (also it makes no sense to do so at this point). Shall we just skip dashboard folder only for sonarcloud check?

Much of this code is still being tested, just not taken into account for coverage.

The issue is that the code is run in a subprocess, which cannot be measured automatically for coverage. This would be the way to go about it: https://coverage.readthedocs.io/en/stable/subprocess.html#configuring-python-for-sub-process-measurement

I saw the tests with playwright but didn't realize that it already covers most of the stuff, while they were merely not reflected by sonarcloud. Thanks for clarifying it. The solution you propose looks good to me. Let's give a try then.

[stefsmeets]

I got a small concern about sonarcloud check. If we merge it as what it is for now, the coverage will drop dramatically. Since we don't want to add unit tests for each function in streamlit (also it makes no sense to do so at this point). Shall we just skip dashboard folder only for sonarcloud check?

Much of this code is still being tested, just not taken into account for coverage.
The issue is that the code is run in a subprocess, which cannot be measured automatically for coverage. This would be the way to go about it: https://coverage.readthedocs.io/en/stable/subprocess.html#configuring-python-for-sub-process-measurement

I saw the tests with playwright but didn't realize that it already covers most of the stuff, while they were merely not reflected by sonarcloud. Thanks for clarifying it. The solution you propose looks good to me. Let's give a try then.

Let's follow up later then. I spent the last hour fighting this, but I can't get it to work at the moment.