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.

Sign up for GitHub

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

Jump to bottom

Adding Section sections cardinality #384

Merged
merged 9 commits into from
Apr 20, 2020
Merged

Adding Section sections cardinality #384

merged 9 commits into from
Apr 20, 2020

Conversation

mpsonntag
Copy link
Contributor

The current PR adds the basic cardinality implementation for Section.sections. It

  • provides an update in the Section.__init__.
  • adds Section.sec_cardinality accessor methods.
  • adds a Section.set_sections_cardinality convenience method.
  • adds and registers a Section sections cardinality validation.
  • provides tests for the implemented features.

The PR refers to issue #361.

@mpsonntag
[section] Add sec cardinality on init
8e50deb
@mpsonntag
[section] Add sec card accessort methods
4f31158
@mpsonntag
[section] Add sec card validation
7569aed 
Also adds a set Section section cardinality
convenience method.
@mpsonntag
[validation] Add sec sec cardinality validation
cd7eef3
@mpsonntag
[format] Add Section.sec_cardinality attribute
416bc27
@mpsonntag
[validation] Add general cardinality function
f6fefe0
@mpsonntag
[test/section] Add sec cardinality tests
4253932 
Adds test for both sec_cardinality setter and
the set_sections_cardinality convenience method.
@mpsonntag
[test/section_integration] Add sec card test
5b431ff 
Add tests for saving and loading of the added
Section.sec_cardinality attribute.
@mpsonntag
[test/validation] Add section sec card test
bbecc62 
Adds tests for the Section sub-section cardinality
validation. Once the ValidationError obj provides
an id attribute, the tests should be refactored.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 76.717% when pulling bbecc62 on mpsonntag:cardSecB into 13fd740 on G-Node:master.

@achilleas-k achilleas-k self-requested a review April 17, 2020 18:18
achilleas-k
achilleas-k approved these changes Apr 17, 2020
View reviewed changes
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

Looks good.

@achilleas-k
Copy link
Member

Want a second review or are we good to merge?

jgrewe
jgrewe approved these changes Apr 20, 2020
View reviewed changes
Copy link
Member

@jgrewe jgrewe left a comment

Choose a reason for hiding this comment

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

lgtm, but still I would like to throw in the type-based restrictions, what do you think?

@jgrewe jgrewe merged commit dd29249 into G-Node:master Apr 20, 2020
@achilleas-k
Copy link
Member

I think we should open an issue about it and see what people think :)

@mpsonntag mpsonntag deleted the cardSecB branch April 22, 2020 17:44
@mpsonntag mpsonntag mentioned this pull request May 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgrewe jgrewe jgrewe approved these changes

@achilleas-k achilleas-k achilleas-k approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mpsonntag @coveralls @achilleas-k @jgrewe