first pass for AP-616 by jason-raitz · Pull Request #4 · BerkeleyLibrary/python-tind-client

jason-raitz · 2026-03-18T19:27:18Z

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.

- commenting out for now to use as template for new tests

awilfox · 2026-03-18T20:12:51Z

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

anarchivist

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

anarchivist · 2026-03-20T14:55:02Z

tind_client/client.py

+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}"'
+


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

If we register the namespace, the library will add that to every record. We could just not register it and manually enter it for the collection.

- and some sensible guard statements - checked for some edge cases - lengthened max-line-length defaults to be a little friendlier

jason-raitz · 2026-03-23T20:57:47Z

Questions

Do we want a standard max-line-length for python?
Do we want to formalize which python tools to use across our various python projects?
(pylint, flake8, mypy, pydoc, uv, linting standards)
For client.write_search_results_to_file(), do we want to return '0' or an error when a Tind response is
successful, but has no matches? Currently it writes nothing to a file and returns '0'.

awilfox

Looking really good, but we should find answers to your questions before merging IMO.

tind_client/client.py

awilfox · 2026-03-23T23:10:32Z

tind_client/client.py


        return recs

+    def write_search_results_to_file(self, query: str = "", output_file_name: str = "tind.xml") -> int:


This is the only non-test line that is above 100 characters; renaming output_file_name to output_file would be enough to bring it under the limit, if we don't want to raise it.

i'd also wager that we can split it into lines which would be sufficient.

awilfox · 2026-03-23T23:11:44Z

.flake8

@@ -1,4 +1,4 @@
 [flake8]
-max-line-length = 100
+max-line-length = 120


not convinced, will elaborate in overall comment.

awilfox · 2026-03-23T23:15:24Z

Questions

Do we want a standard max-line-length for python?

There's a lot of debate in the Python community over this. PEP 8 says 79 characters. I don't think modern Python can be effectively written with a 79 character limit. PyCharm and the Google style guide use 120. I think this is too long, and indeed, with my present eyesight my font size is too large to allow 120 characters to fit on the laptop screen. I think 100 is a fair compromise and is what we used in Willa.

Do we want to formalize which python tools to use across our various python projects?
(pylint, flake8, mypy, pydoc, uv, linting standards)

This would be a great discussion to have at sprint planning.

For client.write_search_results_to_file(), do we want to return '0' or an error when a Tind response is
successful, but has no matches? Currently it writes nothing to a file and returns '0'.

I'm firmly on the side of 0 results not being an error or an exceptional condition.

anarchivist · 2026-03-23T23:24:06Z

tests/test_fetch.py

+def test_write_search_results_to_file_with_malformed_output_filename(
+    client: TINDClient,
+    malformed_filename: str = "  .csv",
+) -> None:
+    """write_search_results_to_file raises ValueError for a malformed output filename."""
+    with pytest.raises(ValueError, match="output_file_name"):
+        client.write_search_results_to_file("", output_file_name = malformed_filename)


how are we determining a malformed filename? i don't remember this being defined in the ticket.

it's not. just an edge case I threw in as a possibility. I could see a case where someone accidentally tries to give csv extension for the xml file.

anarchivist · 2026-03-23T23:25:49Z

tind_client/client.py


        return recs

+    def write_search_results_to_file(self, query: str = "", output_file_name: str = "tind.xml") -> int:


i'd also wager that we can split it into lines which would be sufficient.

anarchivist · 2026-03-23T23:30:59Z

tind_client/client.py

+        with open(output_path, "w", encoding="utf-8") as f:
+            f.write(f'<?xml version="1.0" encoding="UTF-8"?>\n<collection xmlns="{NS}">\n')
+            for record in self._iter_xml_records(query):
+                record_xml = E.tostring(record, encoding="unicode")
+                f.write(record_xml.replace(_NS_DECL, ""))
+                f.write("\n")
+                recs_written += 1
+
+            f.write("</collection>\n")


i'm trying to reconcile what's happening here with what's happening in the test - if i'm understanding this code correctly, we'd be writing the XML declaration with an empty <collection xmlns="http://www.loc.gov/MARC21/slim"></collection> tag pair. is that correct?

good catch! Should I change it to delete the file if no results are written?

tind_client/client.py

anarchivist · 2026-03-23T23:34:57Z

pyproject.toml

+[tool.pylint.format]
+max-line-length = 120


not convinced (just like @awilfox; just making sure we're catching it here, too)

Co-authored-by: Anna Wilcox <AWilcox@Wilcox-Tech.com>

first pass for AP-616

522114a

- 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 self-assigned this Mar 18, 2026

remove search tests

1cb5879

- commenting out for now to use as template for new tests

put client.search() back in

07ce128

anarchivist reviewed Mar 20, 2026

View reviewed changes

added tests

1919ad6

- and some sensible guard statements - checked for some edge cases - lengthened max-line-length defaults to be a little friendlier

jason-raitz marked this pull request as ready for review March 23, 2026 20:56

jason-raitz requested review from anarchivist, awilfox, danschmidt5189 and yzhoubk March 23, 2026 20:58

awilfox reviewed Mar 23, 2026

View reviewed changes

anarchivist reviewed Mar 23, 2026

View reviewed changes

Update tind_client/client.py

ac28514

Co-authored-by: Anna Wilcox <AWilcox@Wilcox-Tech.com>


		return recs

		def write_search_results_to_file(self, query: str = "", output_file_name: str = "tind.xml") -> int:

Conversation

jason-raitz commented Mar 18, 2026

Uh oh!

awilfox commented Mar 18, 2026

Uh oh!

anarchivist left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jason-raitz commented Mar 23, 2026

Questions

Uh oh!

awilfox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awilfox commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Questions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

awilfox commented Mar 23, 2026 •

edited

Loading