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

Issue-408: Calculation referring to additional python module not triggered #457

Merged
merged 6 commits into from
Dec 11, 2017

Conversation

ramonski
Copy link
Contributor

@ramonski ramonski commented Dec 8, 2017

Description of the issue/feature this PR addresses

Linked issue: #408

Current behavior before PR

Calculations using external Python modules failed to calculate

Desired behavior after PR is merged

Calculations using external python modules calculate correctly the results

--
I confirm I have tested this PR thoroughly and coded it according to PEP8 standards.

Copy link
Member

@xispa xispa left a comment

Choose a reason for hiding this comment

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

Please, remove the block entirely instead of commenting it.

# # Get the specs directly from the analysis. The getResultsRange
# # function already takes care about which are the specs to be used:
# # AR, client or lab.
# specs = analysis.getResultsRange()
Copy link
Member

Choose a reason for hiding this comment

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

Yes, you are right, this block of code is no longer used since we introduced format_numeric_result in L273 as per d553f0c#diff-bddafb05eeff20ad5a33aab50565cba2 . You can safely remove this block instead of commenting it.

As an adiditional note, the specs for each analysis are obtained during the analyses list rendering in bika.lims.browser.analyses.py#L758-L770.

In any case, seems evident to me this whole class needs to be refactored asap.

@@ -215,7 +212,7 @@ def calculate(self, uid=None):
'context': self.context},
{'mapping': mapping})
# calculate
result = eval(formula)
result = eval(formula, calculation._getGlobals())
Copy link
Member

Choose a reason for hiding this comment

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

This calculation._getGlobals() is a good trick!

@xispa xispa merged commit 4e1953a into master Dec 11, 2017
@ramonski ramonski deleted the bugfix/issue-408-calcluations-using-python-modules branch December 12, 2017 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants