751_decimal_places_limit #758
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HadronCollider
requested changes
Dec 6, 2025
Collaborator
There was a problem hiding this comment.
- подтяните изменения из dev
- Новые критерии импортируются в модуле https://github.com/moevm/document_insight_system/blob/master/app/main/checks/report_checks/__init__.py (для презентаций в своём), чтобы они могли быть использованы в системе - сейчас в запущенной системе к ним нет доступа и они не используются (= не могут быть запущены для документов)
- установите одинаковые ограничения для критериев (max_decimal_places=2, max_number_of_violations=3)
- если
0<total_violations<max_number_of_violations(т.е. нарушение не обнуляет критерий), необходимо вывести предупреждение, что есть вот такие числа, но их немного
| class PresExcessiveDecimalPlacesCheck(BasePresCriterion): | ||
| description = 'Проверка на избыточное количество десятичных знаков в числах' | ||
| label = "Проверка на избыточное количество десятичных знаков" | ||
| id = 'pres_excessive_decimal_places_check' |
Collaborator
There was a problem hiding this comment.
- неверно определяется/пишется страница для презентации (для отчета забыл проверить, но возможно такая же история)
| - Обычные числа с десятичными знаками (например, -3.14159) | ||
| Не проверяет: | ||
| - IP-адреса (например, 192.168.1.1) | ||
| - Версии ПО (например, 1.2.3.4) |
Collaborator
There was a problem hiding this comment.
Если версия выглядит как дробное число, то засечет (см. скрин проверки у критерия презентаций), но думаю это слишком частный случай - его можно будет учесть в пороге для критериев
Comment on lines
+93
to
+109
| def format_success_message(self): | ||
| if self.type == DocumentType.REPORT: | ||
| document_type = "документе" | ||
| elif self.type == DocumentType.PRESENTATION: | ||
| document_type = "презентации" | ||
| return ( | ||
| f'Проверка пройдена! Все числа в {document_type} имеют допустимое количество ' | ||
| f'десятичных знаков (не более {self.max_decimal_places}).' | ||
| ) | ||
|
|
||
| def format_failure_message(self, total_violations, | ||
| violations_by_location, | ||
| format_page_link_fn=None): | ||
| if self.type == DocumentType.REPORT: | ||
| location_label = "Страница" | ||
| elif self.type == DocumentType.PRESENTATION: | ||
| location_label = "Слайд" |
Collaborator
There was a problem hiding this comment.
Если вся "возня" с enum и if/else только для красивого вывода - это явно лишняя логика, без которой можно спокойно жить, поэтому уберите
| class ReportExcessiveDecimalPlacesCheck(BaseReportCriterion): | ||
| label = "Проверка на избыточное количество десятичных знаков" | ||
| description = 'Проверка на избыточное количество десятичных знаков в числах' | ||
| id = 'excessive_decimal_places_check' |
Collaborator
There was a problem hiding this comment.
Придерживайтесь одного стандартного формата для id критериев (как в критерии для презентаций)
Comment on lines
+21
to
+49
| detected_pages = {} | ||
| total_violations = 0 | ||
|
|
||
| for page_num, page_text in self.file.pdf_file.get_text_on_page().items(): | ||
| lines = re.split(r'\n', page_text) | ||
|
|
||
| page_violations = self.checker.find_violations_in_lines(lines) | ||
|
|
||
| for line_idx, number_str, decimal_places, line in page_violations: | ||
| total_violations += 1 | ||
|
|
||
| if page_num not in detected_pages: | ||
| detected_pages[page_num] = [] | ||
|
|
||
| violation_msg = self.checker.format_violation_message( | ||
| line_idx, line, number_str, decimal_places | ||
| ) | ||
| detected_pages[page_num].append(violation_msg) | ||
|
|
||
| if total_violations > self.max_number_of_violations: | ||
| result_str = self.checker.format_failure_message( | ||
| total_violations, | ||
| detected_pages, | ||
| format_page_link_fn=self.format_page_link | ||
| ) | ||
| result_score = 0 | ||
| else: | ||
| result_str = self.checker.format_success_message() | ||
| result_score = 1.0 |
Collaborator
There was a problem hiding this comment.
С учетом, что эта логика 1 в 1 повторяется в обоих критериях - кажется её можно вынести в одну функцию / метод +- как вы сделали в https://github.com/moevm/document_insight_system/pull/755/files
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Задача #751