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

Add create-meta subcommand #192

Merged
merged 6 commits into from
Oct 28, 2023
Merged

Add create-meta subcommand #192

merged 6 commits into from
Oct 28, 2023

Conversation

qubixes
Copy link
Member

@qubixes qubixes commented Oct 10, 2023

This adds a command to complete the whole pipeline from the command line.

I have added an INI parser to set the specs. An INI file is not ideal, because it has not nesting. However, it is easier to work with than JSON/YAML files, for which you can easily create parsing errors.

fixes #143

metasyn/__main__.py Outdated Show resolved Hide resolved
metasyn/__main__.py Outdated Show resolved Hide resolved
metasyn/__main__.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Samuwhale Samuwhale left a comment

Choose a reason for hiding this comment

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

I have not tested it yet, but the code looks good. Just a few very tiny suggestions, but that's it.

"""Program to create metadata from a dataframe."""
parser = argparse.ArgumentParser(
prog="metasyn create-meta",
description="Create a Generative Metadata Format file from a CSV file.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be nice to explain here that the input CSV has to be a tabular dataset? Not sure if the sentence would be too long then

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't a CSV file always a tabular dataset though?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose they often are, you're right. That said, not everyone might be aware of that, in which case it is a lot more user-friendly to ask for an input tabular dataset, despite being redundant (IMO). It doesn't hurt, that's for sure. But if you like it more this way that is cool too.

metasyn/__main__.py Outdated Show resolved Hide resolved
@qubixes qubixes marked this pull request as ready for review October 13, 2023 08:46
Copy link
Member

@vankesteren vankesteren 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! Let's merge. In the documentation, we might want to include an example config file. Actually, we can now create CLI documentation yeah?

@Samuwhale Samuwhale merged commit 3e2ab85 into main Oct 28, 2023
5 of 6 checks passed
@Samuwhale
Copy link
Collaborator

Looks good! Let's merge. In the documentation, we might want to include an example config file. Actually, we can now create CLI documentation yeah?

I have merged it! There actually already is some CLI documentation, but I'm working on documenting this PR as we speak :)

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.

Multiple entrypoints for CLI
3 participants