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

Context Explorer v2 changes #202

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

acameron13
Copy link

  • Adding subtype tree and context matrix creation to the data-prep-pipeline
  • Editing the get_context_analysis script to return results for all new contexts

args.subtype_tree
)

if context_matrix is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there ever a case where the context matrix being None isn't a gigantic problem? If None is not an acceptable result, should we just let this blow up here so we're aware sooner?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, I just took it out!

'''

#sorting by full subtype name is important so that the level and parent
#assignments don't get jumbled
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think I understand what this comment means. Do you have an example of what the level and parent assignments getting jumbled looks like?

Copy link
Author

Choose a reason for hiding this comment

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

Just added a more in-depth explanation of what I mean!

)

if subtype_tree is not None:
subtype_tree.to_csv(args.output)
Copy link
Contributor

Choose a reason for hiding this comment

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

Like above, I wonder if we should take out this None check. Maybe I'm misunderstanding how this works.

…and causing problems early if things aren't right
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.

2 participants