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

Fix Ruff identified issues for catalog/marc files #8285

Merged
merged 8 commits into from
Sep 18, 2023
Merged

Conversation

hornc
Copy link
Collaborator

@hornc hornc commented Sep 12, 2023

Closes #7886

  • Condenses down the get_subjects from MARC code so that it is easier to read and understand what is being extracted and placed into the various subject type categories.
  • Passes the Ruff complexity test
  • Removes a method and regex to do with 'Aspects' -- unsure what this was for. Undocumented, untested, and the str return value of the function were never used anywhere. Appears to be something that was never fully implemented?
  • Removes some duplication with org subjects, and places appearing duplicated as general subjects or organisations. Tests expectations have been modified to remove the duplicates.
  • More specific exception handling in Binary MARC

Technical

Testing

Screenshot

Stakeholders

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2023

Codecov Report

Merging #8285 (d53e24b) into master (60026bf) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8285   +/-   ##
=======================================
  Coverage   17.08%   17.08%           
=======================================
  Files          75       75           
  Lines        3946     3946           
  Branches      675      675           
=======================================
  Hits          674      674           
  Misses       2842     2842           
  Partials      430      430           

@hornc hornc requested a review from cclauss September 12, 2023 06:12
@hornc hornc changed the title Ruff tests for catalog/marc files Fix Ruff identified issues for catalog/marc files Sep 14, 2023
@hornc hornc added Theme: Development Issues related to the developer experience and the dev environment. [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed] labels Sep 14, 2023
@mekarpeles mekarpeles merged commit 7c8dc18 into master Sep 18, 2023
@mekarpeles mekarpeles self-assigned this Sep 18, 2023
@mekarpeles
Copy link
Member

lgtm, ty

@mekarpeles mekarpeles deleted the cc-marc branch September 18, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Theme: Development Issues related to the developer experience and the dev environment. [managed] Theme: MARC records Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code complexity: read_subjects() in catalog/marc/get_subjects.py is too complex
3 participants