Skip to content

first pass for AP-616#4

Draft
jason-raitz wants to merge 3 commits intomainfrom
AP-616_add_search-to-file_method
Draft

first pass for AP-616#4
jason-raitz wants to merge 3 commits intomainfrom
AP-616_add_search-to-file_method

Conversation

@jason-raitz
Copy link
Contributor

  • removed unused client.search()
  • added client.write_search_results_to_file()
  • added method to iterate through search xml results
  • added some xml fixtures for a first and last result for a sample search as well as the expected output xml for said search.

 - removed unused client.search()
 - added client.write_search_results_to_file()
 - added method to iterate through search xml results
 - added some xml fixtures for a first and last result for a sample
   search as well as the expected output xml for said search.
@jason-raitz jason-raitz self-assigned this Mar 18, 2026
 - commenting out for now to use as template for new tests
@awilfox
Copy link
Member

awilfox commented Mar 18, 2026

Since the eventual goal was to migrate Willa to this as well, I don't think search should be removed.

Copy link
Member

@anarchivist anarchivist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally looks good. what else is left other than the tests?

Comment on lines +19 to +24
NS = "http://www.loc.gov/MARC21/slim"
E.register_namespace("", NS)

# remove namespace that ElementTree adds to records when passed
_NS_DECL: str = f' xmlns="{NS}"'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we stripping the namespace? does it lead to redundancy if we don't?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants