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

Bug fixes for v2/number, v2/number_range, v2/time #416

Merged
merged 13 commits into from
Mar 4, 2021
Merged

Conversation

chiragjn
Copy link
Contributor

@chiragjn chiragjn commented Mar 2, 2021

JIRA Ticket Number

JIRA TICKET: BB-3339, ML-2316, ML-766, ML-2289, ML-2325

Description of change

  • Fixes IndexError for person_name detection and makes the bot message check lenient for unsupported languages
  • Fixes false positive number detection in time detection. Demand a separator for such cases e.g. fail on "145", e.g. "1:45"
  • Fixes number and number range detectors to require the detected spans to have sensible boundaries instead of being any substrings
  • Uncomments and fixes all previously failing tests and updates tests for new changes

  • Minor refactor to ner_v2/api.py
  • Bumps sentry-sdk, django-nose and coverage
  • Changes formatter on logger and removes unused logger
  • Fixes coverage reports
  • Drops unused html, css and js files

Closes: #372
Closes: #397
Closes: #399
Closes: #400
Closes: #401
Closes: #402
Closes: #403
Closes: #404
Closes: #410
Closes: #411

Checklist (OPTIONAL):

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

chiragjn added 7 commits March 2, 2021 12:17
- Fix http response code for internal server errors for v2/text
- Fix false positive number detection in time detection. Demand a
separator for such cases e.g. fail on "145", e.g. "1:45"
- Update sentry-sdk (and refactor requirements.txt)
- Drop unused static html, js, css files to resolve CVE issues
- Fix IndexError for v1/person_name
- Allow bot message check in person_name to be skipped if data for a
language is missing
- Refactor person_name detector code for multi-lingual
it
Also, change logging formatter, drop NLPLogger, and update setup_sentry
@haptik-deployment
Copy link

👍 No lint errors found.

@haptik-deployment
Copy link

UNIT TESTS FAILED... Please resolve issues

@haptik-deployment
Copy link

👍 No lint errors found.

@haptik-deployment
Copy link

👍 No lint errors found.

@haptik-deployment
Copy link

👍 No lint errors found.

@haptik-deployment
Copy link

UNIT TESTS HAVE PASSED... Good To Merge

@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #416 (b86fe55) into develop (5e63269) will decrease coverage by 30.16%.
The diff coverage is 72.05%.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop     #416       +/-   ##
============================================
- Coverage    71.19%   41.02%   -30.17%     
============================================
  Files           78       91       +13     
  Lines         5777     9428     +3651     
============================================
- Hits          4113     3868      -245     
- Misses        1664     5560     +3896     
Impacted Files Coverage Δ
ner_v1/detectors/textual/name/lang_constants.py 100.00% <ø> (ø)
ner_v2/api.py 12.94% <42.10%> (ø)
...er_v2/detectors/temporal/time/en/time_detection.py 68.96% <60.00%> (+0.50%) ⬆️
...tectors/numeral/number/standard_number_detector.py 91.36% <70.58%> (-0.55%) ⬇️
ner_v1/detectors/textual/name/name_detection.py 80.21% <90.90%> (-0.33%) ⬇️
.../numeral/number_range/en/number_range_detection.py 100.00% <100.00%> (+18.18%) ⬆️
...ral/number_range/standard_number_range_detector.py 93.68% <100.00%> (+3.25%) ⬆️
ner_v2/detectors/temporal/time/ta/__init__.py
ner_v1/detectors/textual/text/__init__.py
ner_v1/detectors/pattern/phone_number/__init__.py
... and 82 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a17e27...a818596. Read the comment docs.

@haptik-deployment
Copy link

👍 No lint errors found.

@haptik-deployment
Copy link

UNIT TESTS HAVE PASSED... Good To Merge

@haptik-deployment
Copy link

👍 No lint errors found.

@haptik-deployment
Copy link

UNIT TESTS HAVE PASSED... Good To Merge

@haptik-deployment
Copy link

👍 No lint errors found.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 3, 2021

Kudos, SonarCloud Quality Gate passed!

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@chiragjn chiragjn merged commit a4a6905 into develop Mar 4, 2021
@chiragjn chiragjn deleted the fix_misc_mar2021 branch April 6, 2021 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment