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

Update definition of Cast Op to support casting to/from string #1704

Merged
merged 29 commits into from
Jan 19, 2019

Conversation

raymondxyang
Copy link
Contributor

  • Update the Cast op to support casting to/from string tensors
  • Enable float <-> string test cases

docs/Changelog.md Outdated Show resolved Hide resolved
docs/Changelog.md Outdated Show resolved Hide resolved
docs/Changelog.md Outdated Show resolved Hide resolved
Copy link
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

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

Please only format your changes, otherwise it will be hard for review. Also we need address the precision problem, if the input string is not able to be represented by the target type, what will happen. Also when convert the numeric tensors to string tensors, which representation will be used.

onnx/defs/tensor/defs.cc Outdated Show resolved Hide resolved
@raymondxyang
Copy link
Contributor Author

@houseroad Reverted the format changes in def.cc

docs/Changelog.md Outdated Show resolved Hide resolved
docs/Changelog.md Outdated Show resolved Hide resolved
docs/Changelog.md Outdated Show resolved Hide resolved
Copy link
Contributor

@wschin wschin left a comment

Choose a reason for hiding this comment

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

Nice work!

docs/Operators.md Outdated Show resolved Hide resolved
Copy link
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

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

Looks good, thanks

@houseroad
Copy link
Member

Please resolve the conflict.

@houseroad houseroad added the 1.4 label Jan 19, 2019
@houseroad houseroad merged commit ef028e5 into onnx:master Jan 19, 2019
hariharans29 pushed a commit to hariharans29/onnx that referenced this pull request Aug 15, 2019
…1704)

* Update definition of Cast Op to support convert to/from tensor(string)

* Add test case for casting to/from string; Modify testdata preparation pipeline for unicode dtypes in numpy

* Fix cast test case

* Fix numpy testcase helper

* Fix numpy testcase helper

* Lint code

* Update test coverage

* Update operators.md

* Update numpy string releated scripts

* Fix flake8 error

* Update cast op to support numeric literal values

* Fix cast def

* Update md files

* Update docs for undefined behaviors

* Resolve PR comments

* Fix syntax errors; Refine operator documentation

* Add coverage for Cast testcase

* Update operator doc; Change testcase data generating script using existing unicode string handles

* Update test data

* Update testdata
@magneter
Copy link

Hi ,a similar problem occured to me .
It tips : Unsupported ONNX ops of type: Cast, onnx version is onnx-1.6.0.
It seems ,1.6.0 did not support Cast op.
How Could I solve this ?

jcwchen pushed a commit to jcwchen/onnx that referenced this pull request Sep 23, 2020
…1704)

* Update definition of Cast Op to support convert to/from tensor(string)

* Add test case for casting to/from string; Modify testdata preparation pipeline for unicode dtypes in numpy

* Fix cast test case

* Fix numpy testcase helper

* Fix numpy testcase helper

* Lint code

* Update test coverage

* Update operators.md

* Update numpy string releated scripts

* Fix flake8 error

* Update cast op to support numeric literal values

* Fix cast def

* Update md files

* Update docs for undefined behaviors

* Resolve PR comments

* Fix syntax errors; Refine operator documentation

* Add coverage for Cast testcase

* Update operator doc; Change testcase data generating script using existing unicode string handles

* Update test data

* Update testdata
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.

5 participants