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

feat: Refactor the Python API. #73

Merged
merged 21 commits into from
Jan 31, 2024
Merged

feat: Refactor the Python API. #73

merged 21 commits into from
Jan 31, 2024

Conversation

jetuk
Copy link
Member

@jetuk jetuk commented Nov 28, 2023

Remove the functions and replace with two wrapper classes for Schema and Model. These have basic functionality to deserialise, load and run a model.

Remove the functions and replace with two wrapper classes
for Schema and Model. These have basic functionality to
deserialise, load and run a model.
@jetuk jetuk requested a review from Batch21 November 28, 2023 16:16
Copy link
Contributor

@Batch21 Batch21 left a comment

Choose a reason for hiding this comment

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

Had a quick look through this and I like the approach. A few observations:

  • Python cli is now broken though I suspect that fixing it is already on your TODO list..
  • It would be useful to add some dev dependencies to pyproject toml - pyarrow, h5py, etc..
  • The piecewise-link1 test failed for me. It could not find the mrf1/step-00 component in the output h5 file.

@jetuk
Copy link
Member Author

jetuk commented Nov 30, 2023

Had a quick look through this and I like the approach. A few observations:

* Python cli is now broken though I suspect that fixing it is already on your TODO list..

I hadn't actually noticed this. Do we need a Rust CLI and a Python CLI? If this is staying it needs adding to the Python tests. The pipeline also needs re-enabling.

* It would be useful to add some dev dependencies to pyproject toml - pyarrow, h5py, etc..

Noted!

* The piecewise-link1 test failed for me. It could not find the `mrf1/step-00` component in the output h5 file.

Yes, this is why this is WIP. The test wanted to check the routing through the piecewise link. For this you need to save the sub-node timeseries. This needs support in the OutputMetric enum.

@jetuk
Copy link
Member Author

jetuk commented Jan 23, 2024

This is waiting on #85 for refactoring.

@jetuk jetuk mentioned this pull request Jan 30, 2024
@jetuk jetuk changed the title [WIP] feat: Refactor the Python API. feat: Refactor the Python API. Jan 30, 2024
@jetuk jetuk requested a review from Batch21 January 30, 2024 16:03
jetuk added 2 commits January 30, 2024 16:24
The hdf5 crate is failing and I do not have access to macos
to debug.
README.md Outdated

```bash
python -m pywr
poetry install # install dependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

This will no longer work with the poetry sections taken out of pyproject.toml?

@jetuk jetuk merged commit d3a6517 into main Jan 31, 2024
18 checks passed
@jetuk jetuk mentioned this pull request Jan 31, 2024
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