gafusion / omas

Ordered Multidimensional Array Structure
http://gafusion.github.io/omas
MIT License
30 stars 15 forks source link

Ci testing examples also #149

Closed dboeckenhoff closed 3 years ago

dboeckenhoff commented 3 years ago

Hi, when diving into omas and running examples some do not work, e.g.

$ python omas/examples/omas_dynamic_machine.py

Traceback (most recent call last): File "omas/examples/omas_dynamic_machine.py", line 12, in from omfit_classes.omfit_eqdsk import OMFITgeqdsk ModuleNotFoundError: No module named 'omfit_classes'

This requires omfit which is not part of the omas dependencies. I propose to include the execution of examples in a ci stage.

smithsp commented 3 years ago

I do not think that omas will ever depend on omfit_classes, as that would introduce a circular dependence, where omas is a more fundamental package and OMFIT is more of an accumulator package. So the example, while indicative of how OMFIT and omas could interact, should be robust to omfit_classes not being installed, and the CI should check the examples. Would you be willing to make the example robust to omfit_classes not being installed (it's really only needed for that last part of the example) and create a PR?

orso82 commented 3 years ago

@dboeckenhoff the examples are already part of the CI (see omas/omas/tests/test_omas_examples.py). However, the examples that depend on the omfit_classes are skipped if these are not installed. I have not yet added omfit_classes to the list of requirements simply because we still need to get around to make a conda package for them (they are available on PYPI). For the time being, you can install the omfit-classes with pip install --upgrade omfit_classes

@smithsp I think the circular dependency should not be a problem, since that's something that pip or conda should figure out.

dboeckenhoff commented 3 years ago

@orso82 sorry, yes I did not see test_omas_examples.py - shame on me. By the way: do you guys know (pytest)[https://docs.pytest.org/en/stable/]? It has enormous powers on top of unittest and also integrates e.g. doctests easily.

@orso82 @smithsp I hope with all these issues I am not giving the wrong impression. I really appreciate your work here. I am happy about this intense support from you guys! Thanks again.

dboeckenhoff commented 3 years ago

@orso82 @smithsp I am happy to make the example robust also for users without omfit installation so this does not come up again although as @orso82 points out it is done in test_examples.py already for the unittest. Would you guys grant me some rights to push to an own branch so I can pull-request? Since I would rather not like to fork but if that's the policy, I will.

orso82 commented 3 years ago

@dboeckenhoff I have given you access to the repo (master branch is protected) We appreciate your feedback and we're always happy to learn and improve. I am not familiar with pytest :)