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

Feature/alias elements #94

Closed
wants to merge 29 commits into from
Closed

Feature/alias elements #94

wants to merge 29 commits into from

Conversation

glow-mdsol
Copy link
Member

@iansparks - bundle up of new features and fixes as requested by @anewbigging

handle deprecation of x.getchildren()
test adding alias to multiple elements
move httprettu to a test_require
Harmonised Doc Strings so Sphinx Autodoc works
Partition the Builder examples
Added docstrings to be able to autogen docs
run BLACK formatter over the module
Ran BLACK reformatting on the module (no code changes)
Reformatted documents (specifically around presentation of ..note sections)
Metadata Builders
=================

Note: Any Class with the Prefix **Mdsol** represents a Medidata Rave specific extension
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is rst do you want to make these:

..note:: Any Class with the Prefix **Mdsol** represents a Medidata Rave specific extension ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

OrderNumber=str(self.order_number),
Mandatory=bool_to_yes_no(self.mandatory))
params = dict(StudyEventOID=self.oid, Mandatory=bool_to_yes_no(self.mandatory))
if self._order_number:
Copy link
Collaborator

Choose a reason for hiding this comment

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

0 is an int but would be dropped here. I am not sure that 0 is allowed as an Ordinal anyway, just noting it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great point, updated!

@iansparks
Copy link
Collaborator

Nice work @glow-mdsol Much appreciated.

@@ -461,7 +461,7 @@ def __init__(self, oid, order_number=None, mandatory=False):
def build(self, builder):
"""Build XML by appending to builder"""
params = dict(StudyEventOID=self.oid, Mandatory=bool_to_yes_no(self.mandatory))
if self._order_number:
if self._order_number is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it have to be a cardinal number? I don't know what Rave enforces here. Can you have an Ordinal or 0 or -1 ? ODM recommends numbers starting at 1 but not sure what Rave requires?

Copy link
Member Author

Choose a reason for hiding this comment

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

Rave just seems to parse it as an int.....

@glow-mdsol
Copy link
Member Author

Changes merged across to #95

@glow-mdsol glow-mdsol closed this Nov 13, 2018
@glow-mdsol glow-mdsol deleted the feature/alias_elements branch November 13, 2018 15:12
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.

OrderNumber should be on MeasurementUnit not MeasurementUnitRef
2 participants