113 add markdown to descriptions #118

jcadam14 · 2024-03-25T20:24:21Z

Closes #113

Updated phase_validations to include markdown based on wagtail descriptions https://www.consumerfinance.gov/data-research/small-business-lending/filing-instructions-guide/2024-guide/#4
Updated pytest to be much simpler as it now just needs to do a direct description comparison instead of reformatting due to csv formatting.

Note, currently the pytest is pointing to the raw csv on the branch for cfpb/sbl-content#21. Once those changes are merged in, the pytest can be pointed back to main. So the approval of this PR can wait until that other issue is merged and then I'll reupdate the pytests for final approval.

github-actions · 2024-03-25T20:26:57Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
src/regtech_data_validator
checks.py
phase_validations.py
Project Total

_{This report was generated by python-coverage-comment-action}

hkeeler · 2024-03-26T17:06:54Z

src/regtech_data_validator/phase_validations.py

+                        "When 'action taken' equals 3 (denied), 4 (withdrawn by applicant), or 5 (incomplete), the following fields must all equal 999 (not applicable):\n"
+                        "* 'Interest rate type'\n"
+                        "* 'MCA/sales-based: additional cost for merchant cash advances or other sales-based financing: NA flag'\n"
+                        "* 'Prepayment penalty could be imposed'\n"
+                        "* 'Prepayment penalty exists'\n\n"
+                        "And the following fields must all be blank:\n\n"
+                        "* 'Total origination charges'\n"
+                        "* 'Amount of total broker fees'\n"
+                        "* 'Initial annual charges'"


For these long ones, or maybe just any that aren't one-liners, it might be cleaner to use Python's multiline string, with textwrap.dedent. You'd end up with something like:

from textwrap import dedent ... description="""\ When 'action taken' equals 3 (denied), 4 (withdrawn by applicant), or 5 (incomplete), the following fields must all equal 999 (not applicable): * 'Interest rate type' * 'MCA/sales-based: additional cost for merchant cash advances or other sales-based financing: NA flag' * 'Prepayment penalty could be imposed' * 'Prepayment penalty exists' And the following fields must all be blank: * 'Total origination charges' * 'Amount of total broker fees' * 'Initial annual charges' ```.dedent()

With this model, you can pretty much copy/paste the markdown and not have to worry about all the quotes and \n and that fun.

Ah cool was not familiar with dedent. Just read the api and yeah that would make this code prettier. I'll give that a go. Will possibly need to redo the csv depending on how it spits out stuff but it should match.

src/regtech_data_validator/checks.py

hkeeler

@jcadam14, how are you feeling about all this? Having the multiline strings seems like a bit of an improvement, but it does look a little odd still, and it seems like it's had implications for the CSV file (cfpb/sbl-content#22 (review)). Do you think we should keep it, or revert back to what you had before?

Looks like there are still quite a few descriptions in the old form. If we decide to keep the multiline, we should probably make 'em all like that.

hkeeler · 2024-03-26T23:38:00Z

src/regtech_data_validator/phase_validations.py

+                    description=dedent(
+                        """\
+                        * When 'credit product' does **not** equal 977 (other), 'free-form text field for other credit products' must be blank.
+                        * When 'credit product' equals 977, 'free-form text field for other credit products' must **not** be blank.
+                    """


Did black format it this way, with the offset """s?

hkeeler · 2024-03-26T23:38:56Z

src/regtech_data_validator/phase_validations.py

+                        "* 'Credit purpose' and 'free-form text field for other credit "
+                        "purpose' combined should **not** contain more than three values. "
+                        "Code 977 (other), within 'credit purpose', does **not** count "


This one is still the non-dedent flavor.

I only used dedent on descriptions with actual multiple lines, the \n's. This is a single line description, but split into multiple strings for readability.

hkeeler · 2024-03-26T23:40:21Z

src/regtech_data_validator/phase_validations.py

+                        * When 'interest rate type' does **not** equal 3 (initial rate period > 12 months, adjustable interest), \
+                        4 (initial rate period > 12 months, fixed interest), 5 (initial rate period <= 12 months, adjustable interest), \


It should be valid Markdown without the trailing \. Is that needed to keep in sync with the CSV?

The \ are there to break the string up in the python file so you don't get a single line that looks like "* When 'interest rate type' does not equal 3 (initial rate period > 12 months, adjustable interest), 4 (initial rate period > 12 months, fixed interest), 5 (initial rate period <= 12 months, adjustable interest)..." and runs off the page in the IDE.

Which technically should break our linting but black has issues with long string reformatting.

jcadam14 · 2024-03-27T00:11:06Z

@jcadam14, how are you feeling about all this? Having the multiline strings seems like a bit of an improvement, but it does look a little odd still, and it seems like it's had implications for the CSV file (cfpb/sbl-content#22 (review)). Do you think we should keep it, or revert back to what you had before?

Looks like there are still quite a few descriptions in the old form. If we decide to keep the multiline, we should probably make 'em all like that.

Most descriptions are actually one single line/string, but they're broken up in the python file for readability. Need to make a distinction there. The ones that are actually multiline descriptions have been dedented.

Honestly, the python looks cleaner, but the output to the csv and markdown (before being rendered) is uglier. Depends on if someone using the csv at some point wants a bunch of \n's.

… desc

hkeeler

Looking good. Found a little clump with leftover "s. Founds similar artifacts over in PR cfpb/sbl-content#22 too. I think just the one little fixup and we're good. Thanks for slogging through all this.

src/regtech_data_validator/phase_validations.py

hkeeler · 2024-03-28T01:36:03Z

tests/test_csv_to_code_differences.py

@@ -36,7 +13,7 @@ def test_csv_differences(self):
        ]

        csv_df = pd.read_csv(
-            "https://raw.githubusercontent.com/cfpb/sbl-content/main/fig-files/validation-spec/2024-validations.csv"
+            "https://raw.githubusercontent.com/cfpb/sbl-content/21-update-descriptions-for-markdown/fig-files/validation-spec/2024-validations.csv"


Assuming we'll merge PR cfpb/sbl-content#22 first.

Suggested change

"https://raw.githubusercontent.com/cfpb/sbl-content/21-update-descriptions-for-markdown/fig-files/validation-spec/2024-validations.csv"

"https://raw.githubusercontent.com/cfpb/sbl-content/main/fig-files/validation-spec/2024-validations.csv"

Good catch thank you. After I updated I went through each to check for just this but your eyes start to cross after awhile!

Yep. No doubt. 😄

Closes #113 - Updated phase_validations to include markdown based on wagtail descriptions https://www.consumerfinance.gov/data-research/small-business-lending/filing-instructions-guide/2024-guide/#4 - Updated pytest to be much simpler as it now just needs to do a direct description comparison instead of reformatting due to csv formatting.

jcadam14 added 4 commits March 22, 2024 14:39

Updates to descs for markdown, updated sevs to be capitalized

337c350

Merge branch 'main' into 113-add-markdown-to-errorwarning-descriptions

5f1209c

Updated markdown in descriptions, updated pytest to be much simpler

45cc353

Linting

08b8395

jcadam14 requested review from hkeeler, guffee23 and nargis-sultani March 25, 2024 20:24

jcadam14 linked an issue Mar 25, 2024 that may be closed by this pull request

Add markdown to error/warning descriptions #113

Closed

hkeeler reviewed Mar 26, 2024

View reviewed changes

Added dedent to remove any newline '\n' in strings

5656724

jcadam14 requested a review from hkeeler March 26, 2024 23:04

Removed some straggling quotes

fa5a141

hkeeler reviewed Mar 26, 2024

View reviewed changes

jcadam14 added 2 commits March 26, 2024 21:59

Updated to remove contiuation char and use dedent on any multiline py…

c237ea8

… desc

Linting

24dd093

jcadam14 requested a review from hkeeler March 27, 2024 02:05

hkeeler reviewed Mar 28, 2024

View reviewed changes

Removed extra quotes

f54b264

jcadam14 requested a review from hkeeler March 28, 2024 14:49

Changed test to point back to main sbl-content branch

5580feb

hkeeler approved these changes Mar 28, 2024

View reviewed changes

hkeeler merged commit 30bd513 into main Mar 29, 2024
5 checks passed

hkeeler deleted the 113-add-markdown-to-errorwarning-descriptions branch March 29, 2024 04:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

113 add markdown to descriptions #118

113 add markdown to descriptions #118

jcadam14 commented Mar 25, 2024

github-actions bot commented Mar 25, 2024 •

edited

Loading

hkeeler Mar 26, 2024 •

edited

Loading

jcadam14 Mar 26, 2024

hkeeler left a comment

hkeeler Mar 26, 2024

jcadam14 Mar 27, 2024

hkeeler Mar 26, 2024

jcadam14 Mar 27, 2024

hkeeler Mar 26, 2024

jcadam14 Mar 27, 2024 •

edited

Loading

jcadam14 commented Mar 27, 2024

hkeeler left a comment

hkeeler Mar 28, 2024

jcadam14 Mar 28, 2024

hkeeler Mar 28, 2024

		* When 'interest rate type' does not equal 3 (initial rate period > 12 months, adjustable interest), \
		4 (initial rate period > 12 months, fixed interest), 5 (initial rate period <= 12 months, adjustable interest), \

	"https://raw.githubusercontent.com/cfpb/sbl-content/21-update-descriptions-for-markdown/fig-files/validation-spec/2024-validations.csv"
	"https://raw.githubusercontent.com/cfpb/sbl-content/main/fig-files/validation-spec/2024-validations.csv"

113 add markdown to descriptions #118

113 add markdown to descriptions #118

Conversation

jcadam14 commented Mar 25, 2024

github-actions bot commented Mar 25, 2024 • edited Loading

Coverage report

hkeeler Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hkeeler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcadam14 Mar 27, 2024 • edited Loading

Choose a reason for hiding this comment

jcadam14 commented Mar 27, 2024

hkeeler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 25, 2024 •

edited

Loading

hkeeler Mar 26, 2024 •

edited

Loading

jcadam14 Mar 27, 2024 •

edited

Loading