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

Sector Exposure #166

Closed
wants to merge 16 commits into from
Closed

Sector Exposure #166

wants to merge 16 commits into from

Conversation

a-campbell
Copy link
Contributor

This adds the ability to sum positions by sector given a dictionary or series of symbol to sector mappings.

unmapped_pos = np.setdiff1d(positions.drop('cash', axis=1).columns.values,
symbol_sector_map.keys())
if len(unmapped_pos) > 0:
warn_message = "Warning: Symbols {} have no sector mapping.".format(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add that they will be ignored for the plot below.

@twiecki
Copy link
Contributor

twiecki commented Oct 10, 2015

Looks great, these are just some high-level comments. I'll do a more in-depth review soon.

- See full explanation in tears.create_full_tear_sheet.
sector_alloc : pd.DataFrame
Portfolio allocation of positions. See pos.get_sector_alloc.
show_and_plot : int, optional
Copy link
Contributor

Choose a reason for hiding this comment

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

This kwarg appears to be missing?

@twiecki
Copy link
Contributor

twiecki commented Oct 14, 2015

I rebased the commit. Make sure to pull -f on this branch before making any changes.

ax.set_xlim((df_cum_rets.index[0], df_cum_rets.index[-1]))
ax.set_ylabel('Exposure by sector')


Copy link
Contributor

Choose a reason for hiding this comment

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

It's not returning ax.

@twiecki
Copy link
Contributor

twiecki commented Oct 14, 2015

This looks great, see minor last comments and make sure to force-pull before updating. Thanks @ssanderson for helping with the review.

@twiecki
Copy link
Contributor

twiecki commented Oct 16, 2015

Merged with a89178e. Thanks @acampbell22.

@twiecki twiecki closed this Oct 16, 2015
@gusgordon gusgordon deleted the sector-rollup branch November 4, 2016 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants