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 CLI to run WSIMOD #38

Merged
merged 28 commits into from
Nov 21, 2023
Merged

Add CLI to run WSIMOD #38

merged 28 commits into from
Nov 21, 2023

Conversation

dalonsoa
Copy link
Collaborator

@dalonsoa dalonsoa commented Nov 13, 2023

Adds a CLI for using WSIMOD directly from the terminal.

The main work here has been coming up with an input file structure and processing that can provide enough flexibility to create the appropriate inputs for WSIMOD in a sort of declarative way without becoming too complicated. The chosen format has been YAML, as TOML was looking really odd. The data processing let us run the quickstart demo directly from an input file. An example settings file has been provided.

The most difficult part has been the data processing. Some manipulation has been included which, hopefully, covers many use cases, but ultimately is up to the user to provide input files simple and clean enough such that can be used directly by WSIMOD without too much processing. It is simply not possible to provide custom pre-processing scripts.

TODO

  • Tests (there's a single integration test, for now, to broadly check the functionality.
  • Documentation on how to use this

@dalonsoa dalonsoa linked an issue Nov 13, 2023 that may be closed by this pull request
@dalonsoa dalonsoa marked this pull request as ready for review November 15, 2023 06:44
@dalonsoa dalonsoa requested a review from tg2414 November 15, 2023 09:10
Copy link
Member

@tsmbland tsmbland left a comment

Choose a reason for hiding this comment

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

All looks good to me

This was referenced Nov 16, 2023
Copy link

@AdrianDAlessandro AdrianDAlessandro left a comment

Choose a reason for hiding this comment

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

The CLI seems to run well/as expected, but since new dependencies have been added, all three of the requirements files with the pinned versions need to be updated.

run: python -m pip install -r requirements-dev.txt
run: |
python -m pip install -r requirements-dev.txt
python -m pip install .

Choose a reason for hiding this comment

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

You are not currently installing the pinned versions of the main packages here because they are not specified in the requirements-dev.txt. You can see that pandas etc is being installed at the python -m pip install . step in the CI https://github.com/ImperialCollegeLondon/wsi/actions/runs/6875139142/job/18698156546

@AdrianDAlessandro
Copy link

Just to check, are you intentionally not updating the requirements-dev.txt? Usually, in pip tools, the requirements-dev.txt includes everything from the requirements.txt so to install everything required to run in develop mode you only need pip install -r requirements-dev.txt

@dalonsoa
Copy link
Collaborator Author

No, that's just me being dumb... updated now.

@dalonsoa dalonsoa merged commit 3fed811 into main Nov 21, 2023
21 checks passed
@dalonsoa dalonsoa deleted the cli branch November 21, 2023 11:02
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.

Create CLI for WSIMOD
3 participants