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

First step for building the mermaid flow chart is to parse all the resources and add them as nodes to the flow chart #555

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkgoodrich
Copy link
Contributor

This is just the first step for building the flow chart. It adds all of the nodes that refer to resources. It also adds helpful node attributes like a link to the gnomad_qc resource function in the sphinx documentation, node type, node color, node display name, node id to put in the flowchart code.

There is a very basic outline for the QCFlowchart class that will be filled in more as I add things to this

…sources and add them as nodes to the flow chart
Copy link
Contributor

@ch-kr ch-kr left a comment

Choose a reason for hiding this comment

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

a couple minor questions

Comment on lines +22 to +23
REPOSITORY_ROOT_PATH = str(pathlib.Path(os.path.abspath(__file__)).parent.parent)
sys.path.insert(0, REPOSITORY_ROOT_PATH)
Copy link
Contributor

Choose a reason for hiding this comment

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

cool -- is this to fix the module import errors? asking because I always run something like this

export PYTHONPATH=/Users/kchao/code/gnomad_methods/:/Users/kchao/code/gnomad_qc/

to fix module imports when running scripts locally (so interested in learning about alternatives)

)

for p in paths:
if check_file_exists_raise_error(p, error_if_exists=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

should this have error_if_not_exists=True?

bind_kwargs=bind_kwargs,
)
except (ValueError, DataException, KeyError):
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

should this have a warning message (since it's currently a silent failure)? or are you expecting that some resources will fail

@jkgoodrich jkgoodrich removed the v4.1 label Mar 14, 2024
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.

2 participants