Skip to content

Conversation

@A-Baji
Copy link
Collaborator

@A-Baji A-Baji commented Jan 25, 2023

Copy link
Collaborator Author

@A-Baji A-Baji left a comment

Choose a reason for hiding this comment

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

@zfj1 Please address these changes in a new PR to stage:

Prepare for a new release:

Comment on lines 7 to +9
info % table info
attributes % array of attributes
distinct=false% whether to select all the elements or only distinct ones
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
info % table info
attributes % array of attributes
distinct=false% whether to select all the elements or only distinct ones
info % table info
attributes % array of attributes
distinct=false % whether to select all the elements or only distinct ones


% promote the keys
for iAttr = 1:numel(varargin)
%renamed attribute
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
%renamed attribute
% renamed attribute

if ~isempty(toks)
name = toks{1}{2};
else
%computed attribute
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
%computed attribute
% computed attribute

if ~isempty(toks)
name = toks{1}{2};
else
%regular attribute
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
%regular attribute
% regular attribute

assert(isa(arg, 'dj.internal.GeneralRelvar'),...
'restriction requires a relvar as operand');

% self = init(dj.internal.GeneralRelvar, 'U', {self, arg, 0});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please remove all instances commented code

Suggested change
% self = init(dj.internal.GeneralRelvar, 'U', {self, arg, 0});



% Unions may only share primary, but not secondary, keys
testCase.verifyError(@() count(University.Student() | University.Student()), 'DataJoint:invalidUnion');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
testCase.verifyError(@() count(University.Student() | University.Student()), 'DataJoint:invalidUnion');
testCase.verifyError(@() count(University.Student() + University.Student()), 'DataJoint:invalidUnion');

Switch | to +. We are aware that using the + operator raises a warning suggesting to use | instead. This is was due to a convention that no longer applies. We are working on officially switching to +.

% Unions may only share primary, but not secondary, keys
testCase.verifyError(@() count(University.Student() | University.Student()), 'DataJoint:invalidUnion');
% The primary key of each relation must be the same
testCase.verifyError(@() count(University.A() | University.Student()), 'DataJoint:invalidUnion');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
testCase.verifyError(@() count(University.A() | University.Student()), 'DataJoint:invalidUnion');
testCase.verifyError(@() count(University.A() + University.Student()), 'DataJoint:invalidUnion');


% A basic union
testCase.verifyEqual(count(...
proj(University.Student() & 'student_id<2') | proj(University.Student() & 'student_id>3')),...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
proj(University.Student() & 'student_id<2') | proj(University.Student() & 'student_id>3')),...
proj(University.Student() & 'student_id<2') + proj(University.Student() & 'student_id>3')),...


% Unions with overlapping primary keys are merged
testCase.verifyEqual(count(...
proj(University.Student() & 'student_id<3') | proj(University.Student() & 'student_id>1 AND student_id<4')),...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
proj(University.Student() & 'student_id<3') | proj(University.Student() & 'student_id>1 AND student_id<4')),...
proj(University.Student() & 'student_id<3') + proj(University.Student() & 'student_id>1 AND student_id<4')),...

% Unions with disjoint secondary keys are also merged and filled with NULL
a = University.Student & 'student_id<4';
b = proj(University.Student() & 'student_id>1','"test_val"->test_col');
c = fetch(a | b, '*');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
c = fetch(a | b, '*');
c = fetch(a + b, '*');

@A-Baji A-Baji linked an issue Jan 30, 2023 that may be closed by this pull request
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.

Add support for Unions

3 participants