comaniac commented on a change in pull request #6315:
URL: https://github.com/apache/incubator-tvm/pull/6315#discussion_r474234075
##########
File path: python/tvm/target/target.py
##########
@@ -364,9 +394,11 @@ def create(target_str):
----
See the note on :py:mod:`tvm.target` on target string format.
"""
- if isinstance(target_str, Target):
+ if isinstance(target, Target):
return target_str
- if not isinstance(target_str, str):
- raise ValueError("target_str has to be string type")
+ if isinstance(target, dict):
+ return _ffi_api.TargetFromConfig(target)
+ if isinstance(target, str):
+ return _ffi_api.TargetFromString(target)
Review comment:
I would prefer to support the following use case as well:
```python
target = target.create('./target.json') # Input is a string file path.
```
This case could be supported by using `os` to check if the string is a file
path or not.
In addition, if we would like to support the JSON string, I personally don't
think checking the first character is a proper approach, as you may have spaces
or something else. A better way should be just trying to parse the string with
JSON parser first and use string parser instead if the JSON parser throws an
exception.
##########
File path: tests/python/unittest/test_target_target.py
##########
@@ -80,7 +80,31 @@ def test_target_create():
assert tgt is not None
+def test_target_config():
+ """
+ Test that constructing a target from a dictionary works.
+ """
+ target_config = {
Review comment:
Should also test map-type attributes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
