fix: Mixed Case false positives #1430

briandonahue · 2023-05-03T17:03:17Z

Summary:

Should close #1402. False positives when warning for Mixed Case values such as when numbers and letters are mixed.

Expected behavior:

Values with a letter and numbers such as A102 302-B and those with only numbers like 321 should not trigger a warning.

Please make sure these boxes are checked before submitting your pull request - thanks!

Run the unit tests with gradle test to make sure you didn't break anything
Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
Linked all relevant issues
Include screenshot(s) showing how this pull request works and fixes the issue(s)

...ssor/src/main/java/org/mobilitydata/gtfsvalidator/processor/MixedCaseValidatorGenerator.java

github-actions · 2023-05-03T17:28:50Z

❌ Invalid acceptance test.
New Errors: 0 out of 1426 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1426 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1426 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 272 out of 1426 datasets (~19%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1426 sources (~0 %) are corrupted.
Commit: 9e96ae0
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

briandonahue · 2023-05-16T19:47:33Z

.../tests/src/test/java/org/mobilitydata/gtfsvalidator/processor/tests/MixedCaseSchemaTest.java

+          {"ROUTE 1", false},
+          {"route 1 Boulevard", false},
+          {"Another bad value", false},
+          {"MixedCaseButSingleWord", false},


Feel free to add/suggest more test values.

github-actions · 2023-05-16T20:08:13Z

❌ Invalid acceptance test.
New Errors: 0 out of 1427 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1427 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 134 out of 1427 datasets (~9%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Warnings: 37 out of 1427 datasets (~3%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1427 sources (~0 %) are corrupted.
Commit: aa1e457
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

github-actions · 2023-05-19T16:47:27Z

❌ Invalid acceptance test.
New Errors: 0 out of 1428 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1428 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 134 out of 1428 datasets (~9%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Warnings: 37 out of 1428 datasets (~3%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1428 sources (~0 %) are corrupted.
Commit: 30f09aa
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

github-actions · 2023-05-24T14:58:04Z

❌ Invalid acceptance test.
New Errors: 0 out of 1428 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1428 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 134 out of 1428 datasets (~9%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Warnings: 37 out of 1428 datasets (~3%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1428 sources (~0 %) are corrupted.
Commit: f70a7e7
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

briandonahue · 2023-05-24T15:05:11Z

@davidgamez I've seen this test fail a few times on different PRs - let me know if there is anything I can do / look for to prevent or fix when this happens.

davidgamez · 2023-05-24T19:43:33Z

@davidgamez I've seen this test fail a few times on different PRs - let me know if there is anything I can do / look for to prevent or fix when this happens.

Hi @briandonahue I think the test is doing what is expected to do. In this PR, as in others, there are changes made to the notices logic then the threshold of new/dropped notices is greater than 1%. What we can do is review the test results and make sure the changes that we are planning make sense. You can download the reports and review the example notices.

davidgamez · 2023-05-25T20:09:30Z

@briandonahue I was looking into the acceptance test reports, and it seems that some of the new warnings are coming from the use of full statements in the fields.
Example:

File/field: stop.txt/stop_desc
Value: Book online or call ... for pickup.

@isabelle-dr, do you think we should trigger the mixed case warning rule in the case of stop.txt/stop_desc?

github-actions · 2023-06-02T14:21:07Z

❌ Invalid acceptance test.
New Errors: 56 out of 1428 datasets (~4%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Errors: 14 out of 1428 datasets (~1%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 133 out of 1428 datasets (~9%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Warnings: 92 out of 1428 datasets (~6%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1428 sources (~0 %) are corrupted.
Commit: f70a7e7
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

isabelle-dr · 2023-06-02T15:05:15Z

@briandonahue and @davidgamez I am surprised by the acceptance test results in this PR. It has new errors and dropped errors, which is surprising since the mixed case check is a warning.

Is this because it's out of date with master?

isabelle-dr · 2023-06-02T15:09:20Z

To @davidgamez's question

do you think we should trigger the mixed case warning rule in the case of stop.txt/stop_desc?

I believe the answer is yes, but I am not entirely sure. @tzujenchanmbd, is stop_desc user-facing or is it mostly used between producers and consumers?

github-actions · 2023-06-02T16:17:19Z

❌ Invalid acceptance test.
New Errors: 0 out of 1428 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1428 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 133 out of 1428 datasets (~9%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Warnings: 38 out of 1428 datasets (~3%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1428 sources (~0 %) are corrupted.
Commit: e15cde4
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

briandonahue · 2023-06-02T21:04:49Z

Is this because it's out of date with master?

@isabelle-dr I merged master and it's still having issues. I also apparently closed the PR earlier - no idea how I did that... re-opened.

Might need @davidgamez's help with the test errors...

github-actions · 2023-06-02T21:28:28Z

❌ Invalid acceptance test.
New Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 133 out of 1430 datasets (~9%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Warnings: 38 out of 1430 datasets (~3%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1430 sources (~0 %) are corrupted.
Commit: b5690c4
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

davidgamez · 2023-06-05T12:25:54Z

To @davidgamez's question

do you think we should trigger the mixed case warning rule in the case of stop.txt/stop_desc?

I believe the answer is yes, but I am not entirely sure. @tzujenchanmbd, is stop_desc user-facing or is it mostly used between producers and consumers?

We have sentences in the stop.txt/stop_desc field example: Book online or call ... for pickup This implementation looks for every word in the sentence to have mixed cases. This is why we have a lot of new warnings. We have two options;

Drop the rule for the stop.txt/stop_desc.
Re-think how the rules behave for sentences.

isabelle-dr · 2023-06-05T18:59:45Z

After a conversation during the contributor meeting, we've decided to go with

Drop the rule for the stop.txt/stop_desc.

isabelle-dr · 2023-06-05T19:02:11Z

@briandonahue, would you be able to review the acceptance test report to evaluate if this PR removed most of the false positives, or if they still constitute a big portion?

github-actions · 2023-06-06T18:21:33Z

❌ Invalid acceptance test.
New Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 128 out of 1430 datasets (~9%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Warnings: 45 out of 1430 datasets (~3%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1430 sources (~0 %) are corrupted.
Commit: 802c8ed
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

github-actions · 2023-06-08T19:35:33Z

This contribution does not follow the conventions set by the Google Java style guide. Please run the following command line at the root of the project to fix formatting errors: ./gradlew goJF.

github-actions · 2023-06-08T19:38:05Z

This contribution does not follow the conventions set by the Google Java style guide. Please run the following command line at the root of the project to fix formatting errors: ./gradlew goJF.

github-actions · 2023-06-08T20:07:10Z

❌ Invalid acceptance test.
New Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 3 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 563 out of 1430 datasets (~39%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1430 sources (~0 %) are corrupted.
Commit: fa2bdd4
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

isabelle-dr

When we designed the acceptance tests a while back, this is exactly what we had in mind for how it would be useful :)
I had a quick look at the acceptance test report, this change looks good to me, the number of false positives triggered by this notice dropped significantly with the latest changes.

Thank you for the good work @briandonahue and @davidgamez, I am glad we found a way to retain this notice in the validator!

isabelle-dr · 2023-06-09T09:58:28Z

The last step is a code review by one of the core developers! cc @davidgamez

...ssor/src/main/java/org/mobilitydata/gtfsvalidator/processor/MixedCaseValidatorGenerator.java

github-actions · 2023-06-09T14:28:38Z

This contribution does not follow the conventions set by the Google Java style guide. Please run the following command line at the root of the project to fix formatting errors: ./gradlew goJF.

github-actions · 2023-06-09T14:54:25Z

❌ Invalid acceptance test.
New Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 3 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 563 out of 1430 datasets (~39%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1430 sources (~0 %) are corrupted.
Commit: 34a9c44
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

github-actions · 2023-06-09T17:32:02Z

❌ Invalid acceptance test.
New Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 3 out of 1430 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 563 out of 1430 datasets (~39%) are invalid due to code change, which is above the provided threshold of 1%.
0 out of 1430 sources (~0 %) are corrupted.
Commit: 846c785
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

briandonahue commented May 3, 2023

View reviewed changes

...ssor/src/main/java/org/mobilitydata/gtfsvalidator/processor/MixedCaseValidatorGenerator.java Outdated Show resolved Hide resolved

briandonahue commented May 16, 2023

View reviewed changes

themightychris requested review from davidgamez and isabelle-dr May 23, 2023 20:03

briandonahue added 2 commits May 24, 2023 10:36

change regex to only check 2 or more consecutive letters

bad970b

update with better validation

2c602a8

briandonahue force-pushed the issue/1402/mixed_case_numbers branch from a496c24 to 2c602a8 Compare May 24, 2023 14:36

briandonahue marked this pull request as ready for review May 24, 2023 14:39

briandonahue self-assigned this May 24, 2023

Merge branch 'master' into issue/1402/mixed_case_numbers

bb077bb

briandonahue closed this Jun 2, 2023

briandonahue reopened this Jun 2, 2023

Remove rule from stops.stop_desc

87c4bb3

Testing a more forigiving logic

6c03c31

Add some comments to code generation

cc5657a

briandonahue added 2 commits June 8, 2023 15:38

Fix formatting, clarity

2c27e76

tweak comments

c891b0f

isabelle-dr approved these changes Jun 9, 2023

View reviewed changes

davidgamez reviewed Jun 9, 2023

View reviewed changes

...ssor/src/main/java/org/mobilitydata/gtfsvalidator/processor/MixedCaseValidatorGenerator.java Outdated Show resolved Hide resolved

davidgamez reviewed Jun 9, 2023

View reviewed changes

...ssor/src/main/java/org/mobilitydata/gtfsvalidator/processor/MixedCaseValidatorGenerator.java Outdated Show resolved Hide resolved

remove test/comparison code

62eb02f

briandonahue and others added 2 commits June 9, 2023 10:54

formatting

ed4489c

Merge branch 'master' into issue/1402/mixed_case_numbers

580345f

davidgamez approved these changes Jun 9, 2023

View reviewed changes

davidgamez merged commit eb54951 into master Jun 11, 2023

davidgamez deleted the issue/1402/mixed_case_numbers branch June 11, 2023 22:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Mixed Case false positives #1430

fix: Mixed Case false positives #1430

briandonahue commented May 3, 2023 •

edited

Loading

github-actions bot commented May 3, 2023

briandonahue May 16, 2023

github-actions bot commented May 16, 2023

github-actions bot commented May 19, 2023

github-actions bot commented May 24, 2023

briandonahue commented May 24, 2023

davidgamez commented May 24, 2023 •

edited

Loading

davidgamez commented May 25, 2023

github-actions bot commented Jun 2, 2023

isabelle-dr commented Jun 2, 2023

isabelle-dr commented Jun 2, 2023

github-actions bot commented Jun 2, 2023

briandonahue commented Jun 2, 2023

github-actions bot commented Jun 2, 2023

davidgamez commented Jun 5, 2023

isabelle-dr commented Jun 5, 2023

isabelle-dr commented Jun 5, 2023

github-actions bot commented Jun 6, 2023

github-actions bot commented Jun 8, 2023

github-actions bot commented Jun 8, 2023

github-actions bot commented Jun 8, 2023

isabelle-dr left a comment

isabelle-dr commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

fix: Mixed Case false positives #1430

fix: Mixed Case false positives #1430

Conversation

briandonahue commented May 3, 2023 • edited Loading

github-actions bot commented May 3, 2023

briandonahue May 16, 2023

Choose a reason for hiding this comment

github-actions bot commented May 16, 2023

github-actions bot commented May 19, 2023

github-actions bot commented May 24, 2023

briandonahue commented May 24, 2023

davidgamez commented May 24, 2023 • edited Loading

davidgamez commented May 25, 2023

github-actions bot commented Jun 2, 2023

isabelle-dr commented Jun 2, 2023

isabelle-dr commented Jun 2, 2023

github-actions bot commented Jun 2, 2023

briandonahue commented Jun 2, 2023

github-actions bot commented Jun 2, 2023

davidgamez commented Jun 5, 2023

isabelle-dr commented Jun 5, 2023

isabelle-dr commented Jun 5, 2023

github-actions bot commented Jun 6, 2023

github-actions bot commented Jun 8, 2023

github-actions bot commented Jun 8, 2023

github-actions bot commented Jun 8, 2023

isabelle-dr left a comment

Choose a reason for hiding this comment

isabelle-dr commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

briandonahue commented May 3, 2023 •

edited

Loading

davidgamez commented May 24, 2023 •

edited

Loading