Skip to content

Conversation

@avinashpipalwa
Copy link
Contributor

Made the changes as per the requirement.
Tested this and can see in the post body the Holdings mode which we passed as an input.

Made the changes as per the requirement.
Tested this and can see in the post body the Holdings mode which we passed as an input.
Copy link
Contributor

@vamshimupparaju vamshimupparaju 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 to me

print("PA Component Id: " + component_id)
pa_accounts = [PAIdentifier(id=portfolio)]
pa_benchmarks = [PAIdentifier(id=benchmark)]
pa_accounts = [PAIdentifier(id=portfolio,holdingsmode=holdings)]

Choose a reason for hiding this comment

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

Very minor: please add space after a comma.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

startdate = "20180101"
enddate = "20181231"
frequency = "Monthly"
holdings = "B&H"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this better to make the variable name too as holdingsmode? Also, it may be good to have this variable at line 49 as it is related to accounts and benchmarks.

@srajak8 srajak8 requested a review from ksreeramoj August 8, 2022 11:22
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.

4 participants