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

288 share and create graphs #68

Merged
merged 22 commits into from
Feb 21, 2023
Merged

288 share and create graphs #68

merged 22 commits into from
Feb 21, 2023

Conversation

BendikC
Copy link
Contributor

@BendikC BendikC commented Jan 20, 2023

Completed the issue, created the graphs, fixed the problem with the wrong retrieval from the database, so that part is fine now.

Only thing to note is that I delete the file again in the action "send_metadata" as that is the last place it is used, because I think the only time we will used send_metadata, is when we upload something ourselves and send it. If this is not the case, then the deletion needs to be in a separate action.

Fixes: PerfectFit-project/virtual-coach-issues#288

Checking to see how functionality works, creation of a simple graph. Local changes to rules are not pushed, but include changes to call these actions from somewhere easier than the middle of the dialog
Branch wasnt up to date
Still needs to be cleaned up, the other two figures need to be added, and PA needs to be dealt with
Subplots work, but still not particularly pretty. Neither is the code, yet
Completed show_barchart_difficult situations, now same logic should be implemented in pa, and a way to customize the file path needs to be found
This avoids the filepath being hardcoded so that we can use the upload_file action after every action that needs it
Now gets the correct results from the database
Checkstyle fixes
@wbaccinelli wbaccinelli self-requested a review January 23, 2023 08:53
Copy link
Contributor

@wbaccinelli wbaccinelli left a comment

Choose a reason for hiding this comment

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

Good job! I left some suggestions.
As a general comment, it would be good to use type annotation when defining new functions in the helper. It helps using them correctly.

Rasa_Bot/actions/actions_common.py Outdated Show resolved Hide resolved
Rasa_Bot/actions/actions_relapse_dialogs.py Outdated Show resolved Hide resolved
Rasa_Bot/actions/actions_relapse_dialogs.py Outdated Show resolved Hide resolved
Rasa_Bot/actions/helper.py Outdated Show resolved Hide resolved
Rasa_Bot/actions/helper.py Outdated Show resolved Hide resolved
BendikC and others added 10 commits February 14, 2023 12:38
Applying suggestions, but still need to deal with additional feedback

Co-authored-by: Walter Baccinelli <100706999+wbaccinelli@users.noreply.github.com>
Wrong indentation
Pull request suggestions considered and code has been revised
Locally prospector was passing so weird for it to fail here
Cant run pytype locally so might have to do this more times
Fixes proposed by Kristell, there are a few more that require functional changes that need to be implemented
Kristell's suggestions to merge lapse and relapse in the graphs
Another of kristells suggestions
Import wasn't carried over in the merge. Probably something I missed when resolving the conflicts
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

9.3% 9.3% Coverage
0.0% 0.0% Duplication

@wbaccinelli wbaccinelli merged commit a2cbc30 into main Feb 21, 2023
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.

Share_and_create_graphs
3 participants