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

Move Galaxy tools to the eBCSgen repository #118

Open
wants to merge 3 commits into
base: ebcsgen-3.0
Choose a base branch
from

Conversation

mopichalova
Copy link
Collaborator

@mopichalova mopichalova commented Nov 25, 2024

Moved the top-level API functions from the galaxytools repository to /api directory in this repository, the eBCSgen repository.
No modifications were made to the scripts or the original repository.

Close #115

@xtrojak xtrojak self-requested a review November 26, 2024 13:32
Copy link
Collaborator

@xtrojak xtrojak left a comment

Choose a reason for hiding this comment

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

We don't want to move Galaxy tools here - only the top-level API functions (.py files). All the XML files, test data, or shed file don't belong here. And also it does not belong to Galaxy subdirectory (this should be actually removed one day when all "local" issues are resolved - e.g. properly sharing visualisations).

@mopichalova
Copy link
Collaborator Author

We don't want to move Galaxy tools here - only the top-level API functions (.py files). All the XML files, test data, or shed file don't belong here. And also it does not belong to Galaxy subdirectory (this should be actually removed one day when all "local" issues are resolved - e.g. properly sharing visualisations).

Thank you for your feedback regarding the structure of the repository. I have made the necessary changes as per your suggestions. Moved the top-level API functions to directory in the root of this repository (commit 5eca9c2) and removed all xml files, test data and the Galaxy subdirectory, ensuring that only the relevant Python files remain (commit 5eca9c2).

@xtrojak
Copy link
Collaborator

xtrojak commented Dec 9, 2024

This is a good start, but I would improve it in following ways:

  1. Rename the api folder to eBCSgen/API - we want it to be inside the Python package to make it available for imports
  2. Define the argument parsing using argparse inside if __name__ == '__main__': section to allow calling it from the command line, but define the top level function separately - a random example demonstrating this.
  3. (optional) Create __init__.py file in the API folder alongside with all the top-level scripts, importing all top-level functions in one place. This can simplify importing the functions when using the eBCSgen library. Another random example.
  4. Update readthedocs documentation - maybe there is some argparse integration, you have to check.

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