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

Conversion to support both Python 2.7 and Python 3.5 #5

Merged
merged 22 commits into from
Jul 25, 2021

Conversation

danizen
Copy link
Contributor

@danizen danizen commented Sep 21, 2018

Hi,

I'm hoping this addresses #3 and #4. I'm also hoping you are willing to act as a host to work on this software - I understand you've probably moved on, but if you can tell me how to run the various pieces, we can begin to get it back in working order.

If you look at my github profile, you'll be able to see my interest. I've brought the OCCS/AB branch of the National Library of Medicine to Python, and as we upgrade our ILS, our cataloging/meta-data workflow depends on my getting this or some other well-supported MARC metasearch working with Python 3.

@danizen
Copy link
Contributor Author

danizen commented Sep 21, 2018

Well, it isn't working as well as I thought. tox error. Adding a travis-ci.org build is helping. Give it a couple of days.

@@ -136,7 +136,7 @@ class ModifiableObject:
modifiers = []

def __getitem__(self, k):
if (type(k) == types.IntType):
if (type(k) == int):
Copy link
Contributor

Choose a reason for hiding this comment

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

PEP8: Use isinstance()

@@ -2,11 +2,10 @@
#!/usr/local/bin/python2.3

try:
from cStringIO import StringIO
from io import StringIO
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above.

This code says do X, if that does not work, do X again.

Co-authored-by: Christian Clauss <cclauss@me.com>
@danizen
Copy link
Contributor Author

danizen commented Feb 12, 2021

Timely, they just got our new Alma Z39.50 server running - I will get back to this.

#!/usr/local/bin/python2.3
from io import StringIO
Copy link
Contributor

Choose a reason for hiding this comment

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

In Python 2 io.StringIO != StringIO.StringIO

Suggested change
from io import StringIO
try:
from StringIO import StringIO # Python 2
except ImportError:
from io import StringIO # Python 3

@mogul
Copy link

mogul commented Jul 22, 2021

Hi there @cclauss and @danizen, what are the prospects for getting this PR merged any time soon?

@cclauss
Copy link
Contributor

cclauss commented Jul 22, 2021

I am not a maintainer and have no rights to merge pull requests.

@mogul
Copy link

mogul commented Jul 23, 2021

My apologies! 😊

@asl2, are you able to respond?

@asl2 asl2 merged commit 9aeffea into asl2:master Jul 25, 2021
@asl2
Copy link
Owner

asl2 commented Jul 25, 2021

My apologies for not responding earlier. I haven't done anything Z3950-related (or indeed library-related) in a while.

This is fine as far as it goes, but when I try to run under Python3 I get the following traceback:

Traceback (most recent call last):
File "test1.py", line 33, in
main()
File "test1.py", line 28, in main
for r in get_results():
File "test1.py", line 8, in get_results
conn = zoom.Connection ('z3950.loc.gov', 7090)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/zoom.py", line 276, in init
self.connect()
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/zoom.py", line 302, in connect
optionslist = options, **initkw)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/z3950.py", line 511, in init
('initRequest', InitReq), 'initResponse')
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/z3950.py", line 529, in transact
b = self.encode_ctx.encode (APDU, to_send)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 622, in encode
spec.encode (self, data)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 1192, in encode
ctyp.encode (ctx, val)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 858, in encode
self.typ.encode (ctx, val)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 796, in encode
self.encode_val (ctx, val)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 1504, in encode_val
typ.encode (ctx, v)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 858, in encode
self.typ.encode (ctx, val)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 796, in encode
self.encode_val (ctx, val)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 1089, in encode_val
ctx.bytes_write (val)
File "/usr/local/lib/python3.6/dist-packages/PyZ3950/asn1.py", line 633, in bytes_write
raise EncodingError("Bad type to bytes_write")

Perhaps I've done something obvious wrong, but my thought was that PyZ3950 relied on Python 2's identity of string and bytes and would need somewhat more work for a port to Python 3.

I'll try to free up some time to look at this more closely.

@mogul
Copy link

mogul commented Jul 26, 2021

I saw the commit fixing the tests. Thanks so much for taking care of this!

@cclauss
Copy link
Contributor

cclauss commented Jul 26, 2021

Please see #6 for other Py3 changes needed.

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.

4 participants