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

Populate with Python 3.6: "TypeError: expected bytes, str found" #1187

Closed
vered1986 opened this issue Jan 16, 2018 · 9 comments
Closed

Populate with Python 3.6: "TypeError: expected bytes, str found" #1187

vered1986 opened this issue Jan 16, 2018 · 9 comments
Labels
major bug Issues that silently cause incorrect results, break installation on common environments, etc.

Comments

@vered1986
Copy link
Contributor

Hi,

I'm trying to load parameters of a model using populate:

w.populate(load_from, '/w')

where w is a parameter and load_from is the file path string.

I get the following exception:

  File "_dynet.pyx", line 563, in _dynet.Parameters.populate
  File "_dynet.pyx", line 575, in _dynet.Parameters.populate_from_textfile
  File "stringsource", line 15, in string.from_py.__pyx_convert_string_from_py_std__in_string
TypeError: expected bytes, str found

I saved the parameters using the following code:

w.save(save_to, '/w')

where save_to is the same string as load_from.

I work with python 3.6 and I suspect that the exception stems from the difference between strings in python 2 and 3: http://docs.cython.org/en/latest/src/tutorial/strings.html.

Thanks!

@vered1986
Copy link
Contributor Author

It seems that removing the string requirement for the arguments of populate_from_textfile solves the problem in python3.6 (i.e. I defined it as def populate_from_textfile(self, fname, key=""), similarly to the way it is defined for ParameterCollection and LookupParameters).

@neubig
Copy link
Contributor

neubig commented Jan 16, 2018

Ahh, yes, this is a common problem. Would you mind sending a pull request fixing the problem quickly?

@xunzhang I'm not sure why our unit tests didn't catch this though. Perhaps we need to add an IO unit test to Python?

@neubig neubig added the major bug Issues that silently cause incorrect results, break installation on common environments, etc. label Jan 16, 2018
@vered1986
Copy link
Contributor Author

Sure, but I haven't tested it on python 2... I guess it should be fine assuming that ParameterCollection.populate_from_textfile and LookupParameters.populate_from_textfile work there.

@xunzhang
Copy link
Collaborator

I added some tests in the above pull request. But I don't have a python3 environment, @vered1986 could you help test it?

@vered1986
Copy link
Contributor Author

Sure, what should I run? (I guess this? https://github.com/clab/dynet/pull/1189/files)

@xunzhang
Copy link
Collaborator

Yes, https://raw.githubusercontent.com/xunzhang/dynet/5b6cf0e0b5f76ab534943067676ab927c4ca6107/tests/python/test.py . You should get an error with current master and pass it with the modification of your pull request.

@vered1986
Copy link
Contributor Author

Yes, works as expected.

Output for master:

[dynet] random seed: 267491601
[dynet] allocating memory: 512MB
[dynet] memory allocation done.
....Reading clusters from cluster_file.txt ...
Read 10 words in 5 clusters (0 singleton clusters)
...........E........................
======================================================================
ERROR: test_save_load (__main__.TestIOPartial)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test.py", line 311, in test_save_load
    a.populate(self.file, "/a")
  File "_dynet.pyx", line 563, in _dynet.Parameters.populate
  File "_dynet.pyx", line 575, in _dynet.Parameters.populate_from_textfile
  File "stringsource", line 15, in string.from_py.__pyx_convert_string_from_py_std__in_string
TypeError: expected bytes, str found

----------------------------------------------------------------------
Ran 40 tests in 0.056s

FAILED (errors=1)

Output for pull request:

[dynet] random seed: 3442494103
[dynet] allocating memory: 512MB
[dynet] memory allocation done.
....Reading clusters from cluster_file.txt ...
Read 10 words in 5 clusters (0 singleton clusters)
....................................
----------------------------------------------------------------------
Ran 40 tests in 0.067s

OK

@xunzhang
Copy link
Collaborator

looks good

@xunzhang
Copy link
Collaborator

Fixed via #1188.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major bug Issues that silently cause incorrect results, break installation on common environments, etc.
Projects
None yet
Development

No branches or pull requests

3 participants