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

New form retrieval #15

Merged
merged 19 commits into from
Aug 1, 2024
Merged

New form retrieval #15

merged 19 commits into from
Aug 1, 2024

Conversation

anjus1313
Copy link
Collaborator

@anjus1313 anjus1313 commented Jul 25, 2024

Added getFormJson function to retrieve the entire form information when dataset, orgunit and period are given. This information is now used to generate key value pairs as well as get the list of dataElements and categories for doing field name correction using Levenshtein distance.

Images that have a different field name in metadata from the one on tally sheet:

  1. EVERYTHING works but field looks different from tally sheet
    Tally sheet name has been modified to the one in metadata by correct field names function:
    preg_(forvac)vspreg_nameissue_2

Fields in metadata
preg_(forvac)vspreg_nameissue
preg_(forvac)vspreg_nameissue_3

  1. DOES NOT WORK
    Pop1: Resident (Population 1 in metadata) gets changed to Pop1: Resident Discontinued user with correct field names
    Fields retrived from metadata
    pop1_res_pop1_so_Leven_doesnot_work_1
    pop1_res_pop1_so_Leven_doesnot_work_2

@ginic
Copy link
Contributor

ginic commented Jul 26, 2024

Thanks, for the detailed examples, @anjus1313 ! It's good to know about this issue. I tried to test this with your corresponding PR in the front end today, but I was having trouble getting things working in a way that made sense with the multi-page. Let's test it together next week.

@ginic ginic force-pushed the new-form-retrieval branch from cf314e5 to 3bfa03f Compare July 26, 2024 20:13
@anjus1313 anjus1313 force-pushed the new-form-retrieval branch from 3bfa03f to 04423c9 Compare July 31, 2024 14:12
@anjus1313
Copy link
Collaborator Author

I have added all code changes from the correct_field_names-correction branch in MSR-OCR-streamlit repo into this branch.

Now the correct field names, generate key value pairs and upload to DHIS2 functionality works with multi-page functionality.

No major code changes from the last commit [e4a1122] in correct_field_names-correction branch by @ginic except for variable name changes and one line change to update session states only for tables that have been edited.

@rdziewietin
Copy link
Contributor

I added the changes we discussed in office hours today. So now the way it should work is:

  • The form data is saved when the user presses confirm data
  • The form data is saved when the user presses correct field names
  • The form data is NOT saved when the user presses generate key-value pairs
  • The field names are not changed for the user if the session state data is the correct names but the field view is not

Anju and I talked about removing the correct field names button, but we're leaning towards not doing that because it requires header data to be entered and it seems jarring for the user to see it change. However, with the async form data, maybe we should revisit?

Copy link
Contributor

@ginic ginic left a comment

Choose a reason for hiding this comment

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

Looks great! I was able to test it with a few of the preventive vaccination images!

categoryOptionCombos = categoryCombos_to_name_to_id[categoryCombo]
category_id = categoryOptionCombos[category]

if cell_value is not None and cell_value!="-" and cell_value!="":
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, good check. I wonder if we should even remove those during preprocessing, like the same time we evaluate the math expressions. But that's a task for later.

app_llm.py Outdated
@@ -5,6 +5,7 @@

import requests
import streamlit as st
from pip._vendor.requests.auth import HTTPBasicAuth
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's import the requests version of this instead. The single _ import usually means that the module is only for internal use (see https://realpython.com/python-double-underscore/), so it might not be safe to rely on.

Suggested change
from pip._vendor.requests.auth import HTTPBasicAuth
from requests.auth import HTTPBasicAuth

@ginic
Copy link
Contributor

ginic commented Aug 1, 2024

@anjus1313 , @rdziewietin, @ByteYJ I made several updates to the new authentication code, so I'd appreciate it if someone besides me tested everything out before we merge to main. Everything looks good to me though!

@ByteYJ
Copy link
Collaborator

ByteYJ commented Aug 1, 2024

@ginic The code works well for all authentication steps. Thanks for making changes so quickly in many parts of the code.

@ginic ginic merged commit ec2b533 into main Aug 1, 2024
5 checks passed
@ginic ginic deleted the new-form-retrieval branch August 5, 2024 18:39
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.

4 participants