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

Revise the exporters API #43

Merged
merged 2 commits into from
Jan 31, 2019
Merged

Revise the exporters API #43

merged 2 commits into from
Jan 31, 2019

Conversation

izeigerman
Copy link
Member

No description provided.

@izeigerman izeigerman changed the title Rewise the exporters API Revise the exporters API Jan 31, 2019
return assembler_cls
def _export(model, interpreter):
model_name = type(model).__name__
assembler_cls = _get_assembler_cls(model_name)
Copy link
Member

Choose a reason for hiding this comment

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

I know it's probably my code, but maybe move logic of building model_name into _get_assembler_cls function?

@@ -45,9 +45,7 @@ def interpret(self, expr):
os.path.dirname(__file__), "linear_algebra.java")
top_cg.add_code_lines(utils.get_file_content(filename))

return [
(self.model_name, top_cg.code),
]
Copy link
Member

Choose a reason for hiding this comment

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

Nice!!

for model_name, code in self.exporter.export():
file_name = os.path.join(self._resource_tmp_dir,
"{}.java".format(model_name))
model_name = "Model"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe move it to the class level and reuse in predict() method?

@coveralls
Copy link

coveralls commented Jan 31, 2019

Coverage Status

Coverage decreased (-0.03%) to 95.534% when pulling 6a37a0f on iaroslav/exporter_api into 2d0105b on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 95.534% when pulling 811700d on iaroslav/exporter_api into 2d0105b on master.

@krinart krinart merged commit d536e0f into master Jan 31, 2019
@krinart krinart deleted the iaroslav/exporter_api branch January 31, 2019 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants