Skip to content

Conversation

@mcflugen
Copy link
Member

This pull request adds a simple command to create a template BMI implementation Python file that a user can fill in. It doesn't do too much right now but can be extened.

To get help,

$ bmipy-render --help

To create a BMI template implementation,

$ bmipy-render ByBmi > mybmi.py

@kbarnhart This isn't everything that you were wanting but I think it could at least provide a place for you to add addition functionality as you need it.

@mdpiper Maybe this is something you would be interested in as well?

@mdpiper
Copy link
Member

mdpiper commented Apr 30, 2019

@mcflugen Definitely. Thank you! I'll use this in the BMI Live clinic.

@kbarnhart
Copy link

kbarnhart commented Apr 30, 2019

@mcflugen. this looks great.

I couple of comments:

  1. I tried it and it didn't fail 🎉
  2. When I tried it I got:
from bmipy import Bmi
import numpy


class update(Bmi):

    def finalize(self) -> None:
        """Perform tear-down tasks for the model.

        Perform all tasks that take place after exiting the model's time
        loop. This typically includes deallocating memory, closing files and
        printing reports.
        """
        raise NotImplementedError("finalize")

which indicates to me that filling in the correct class name is not working (and the last function, update) is being used instead.

  1. How terrible is it to make it Py27 compatible (e.g. no --> float). If I decide to just drop Py27 for terrainbento... this perhaps doesn't even matter. If you think it doesn't matter, I'm happy deciding it doesn't matter!

A couple of additional notes on additional functionality that would be great (@mdpiper let me know what you think).

  1. An example test
  2. decorators on bmi functions indicating what sorts of grid types are relevant. Then passing --unstructured=True would result in those functions that are used with unstructured getting the standard NotImplementedError("function name") but the others getting something like NotImplementedError("function name not implemented for grid types of kind ....")

@kbarnhart
Copy link

Running Python 3.6.8 I had issues with Tuple[str].

e.g.

    def get_input_var_names(self) -> Tuple[str]:
NameError: name 'Tuple' is not defined

Once I removed those, I had no errors.

@mcflugen
Copy link
Member Author

@kbarnhart Based on your comments, I've made a few improvements:

  • Added a --no-hints option that will remove the type hint annotation
  • ImportTuple from typing if hints are being used
  • Fixed the class being name "update" bug

@kbarnhart
Copy link

I can confirm that all three of your improvements worked for me!
🌋 🎉

@kbarnhart
Copy link

Since you are on a roll @mcflugen (🌟 🎉 ) I thought I'd write out my vision for adding a test example templates...

For each command, if --make-example-tests flag was included, then the following would be appended to the docstring.

Examples
--------
>>> from model_module  import ModelName
>>> model = ModelName.initialize("name_of_input_file.yml")
>>> model.{{ func_name }}( {{ func_args }} ) == meaningful_test

where {{ func_name} } and {{ func_args }} are replaced by the correct results from inspect.

This way a user could very easily find replace on model_module and ModelName. They would then have to use their brain to replace the default func_args with something meaningful for their model, and similarly make meaningful_test something sensible.

@mcflugen
Copy link
Member Author

mcflugen commented May 1, 2019

@kbarnhart I like your idea about adding example doctests. I will try to add that but as a separate pull request.

@mcflugen mcflugen merged commit 654e2e8 into master May 1, 2019
@mcflugen mcflugen deleted the mcflugen/add-cli branch May 1, 2019 16:11
@kbarnhart
Copy link

Sounds good @mcflugen. I thought I'd at least write down what I was thinking of while I was thinking about it so I don't have to worry about forgetting....

Thanks for making this. As it stands its pretty spiffy!

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.

4 participants