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

Fix TypeError: "Can't pickle objects in acquisition wrappers" (Calculation) #340

Merged
merged 3 commits into from
Nov 1, 2017

Conversation

xispa
Copy link
Member

@xispa xispa commented Oct 31, 2017

Fixes #322

The assignment of the whole Calculation object in a metadata column getCalculation was causing a "TypeError: Can't pickle objects in acquisition wrappers".

With this Pull Request, getCalculation metadata column has been replaced by getCalculationUID in bika_analysis_catalog. This metadata field is only used in analyses listing to flag an analysis as it result needs to be calculated.

I've done some functional testing and calculations in manage results view work as expected, also if old calculations are edited (generating new versions). Afak, tests for calculations were the only thing that was failing in #320 .

Requires upgrade step v1.1.2

@xispa xispa mentioned this pull request Oct 31, 2017
@xispa xispa requested a review from ramonski October 31, 2017 23:50
@@ -459,7 +459,7 @@ def folderitem(self, obj, item, index):
item['class']['retested'] = 'center'
item['result_captured'] = self.ulocalized_time(
obj.getResultCaptureDate, long_format=0)
item['calculation'] = obj.getCalculation and True or False
item['calculation'] = obj.getCalculationUID
Copy link
Contributor

Choose a reason for hiding this comment

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

Better change this line to obj.getCalculationUID and True or False so that it still contains a boolean type instead of an empty string '' or an UID '0666cd13ce844451b24f32ec8f5cd883'

Copy link
Member Author

Choose a reason for hiding this comment

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

The value for this field is only used to check if the analysis requires calculation, so I thought there was any difference. In fact, I thought was better to set the UID, cause it would be used later with coffescript to get the calculation directly. But yes, you are right, will do the change

@@ -1,6 +1,6 @@
<?xml version="1.0"?>
<metadata>
<version>1.1.0</version>
<version>1.1.2</version>
<dependencies>
Copy link
Contributor

@ramonski ramonski Nov 1, 2017

Choose a reason for hiding this comment

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

Why the jump directly to 1.1.2 and not 1.1.1?

Copy link
Member Author

Choose a reason for hiding this comment

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

I missed to set the metadata's version to 1.1.1 in my previous upgradestep :(

Copy link
Contributor

@ramonski ramonski left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, I'm happy to have this now in code

@ramonski ramonski merged commit d408ab8 into senaite:senaite-integration Nov 1, 2017
@xispa xispa deleted the i322 branch November 27, 2017 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants