ENH: Add examples to the template. #46

jhlegarreta · 2018-12-23T22:22:22Z

Add a hint to foster writing examples for the contributed classes.

Resolves #34.

Add a hint to foster writing examples for the contributed classes.

jhlegarreta · 2018-12-23T22:26:26Z

A few comments:

Not sure whether the folder will get contents in most cases. If you feel that the risk of leaving the folder empty is greater than the benefit of having the template, discard the PR. Another option would be to just leave the call to itk_module_examples() in the CMakeLists.txt.
Not sure about the right place to set the itk_module_examples() call in the CMakeLists.txt (i.e. building inside ITK vs. as a remote). I just took this as a recent example.
Not sure about any naming convention we should be using for the examples.
As opposed to the tests, the examples may become involved, so I preferred not do write any code in them, and just write some instructions. Feel free to modify.
The same applies to the examples/CMakeLists.txt dependencies.

jhlegarreta · 2018-12-24T16:08:55Z

I've further thinking about this. In absence of any elaborated examples, a fairly easy solution, and a nice and much needed addition could be to write a performance benchmark. Considering the way the ITKPerformanceBenchmarking is structured, an example reporting the performance could fit into that framework. Otherwise, if we think that performance benchmarks should be made more explicit and deserve a separate module, we may need to further think how we can propose this. In either case, I'd be happy to work on that.

Also, the example pointed in #34 embeds the example binary data within the same folder, instead of using the test/Input. May be another folder should be created within the examples folder to host the data. Locating binary data in test/Input that is not actually used by tests may be misleading, even if it allows to keep all data in one place. Suggestions are welcome.

dzenanz

I think that adding the example to the module template is useful.

dzenanz · 2018-12-24T16:38:45Z

{{cookiecutter.project_name}}/examples/CMakeLists.txt

+
+find_package(ITK REQUIRED
+  COMPONENTS
+    ITKImageIO


This is problematic if the example is built as part of ITK. See this.

@fbudin69500 do you think we should add this to the module template?

This seems difficult to address cleanly for now as the template would have to explicitly instantiate certain IOs and it is not possible to know ahead of time which IOs should be instantiated. There could be hints in the CMakeLists.txt with commented out lines of code with the IO names that the developer could either uncomment or remove, but that doesn't seem ideal. @dzenanz : Was an issue created in the issue tracker for that Meta module IO issue?

I just created an issue: InsightSoftwareConsortium/ITK#359

We could apply solution from ITKMontage:
https://github.com/InsightSoftwareConsortium/ITKMontage/blob/v0.8.2/examples/CMakeLists.txt#L4-L22

jhlegarreta · 2018-12-24T16:52:19Z

{{cookiecutter.project_name}}/examples/CMakeLists.txt

@@ -0,0 +1,12 @@
+cmake_minimum_required(VERSION 3.10.2)
+project({{ cookiecutter.module_name }}Examples)


Do we need to add CXX or does this help CMake somehow exclude any Python example file that is added along?

Python examples don't need to be built -- the default include CXX -> this is good.

🤔 So do we need to use

project({{ cookiecutter.module_name }}Examples CXX)

explicitly then?

jhlegarreta · 2018-12-24T16:53:45Z

{{cookiecutter.project_name}}/examples/CMakeLists.txt

@@ -0,0 +1,12 @@
+cmake_minimum_required(VERSION 3.10.2)
+project({{ cookiecutter.module_name }}Examples)


Also, do we need to enable and add tests for the examples?

If the example does something, we can use it to write a test, see this. Having test data in there is harder to properly support, that's why I removed it.

thewtex · 2019-01-03T03:58:35Z

Awesome @jhlegarreta ! This is a great contribution. Examples are essential.

For the testing data, perhaps we can use the CMake FetchContent module? It requires CMake 3.11, but we could just add cmake_minimum_required(VERSION 3.11) in examples/CMakeLists.txt only.

jhlegarreta · 2019-01-03T15:39:34Z

Awesome @jhlegarreta ! This is a great contribution. Examples are essential.

For the testing data, perhaps we can use the CMake FetchContent module? It requires CMake 3.11, but we could just add cmake_minimum_required(VERSION 3.11) in examples/CMakeLists.txt only.

🤔 Not sure how the CMake FetchContent module plays a role here or how does it fit with having tests for the examples.

thewtex · 2019-01-04T03:56:35Z

Not sure how the CMake FetchContent module plays a role here or how does it fit with having tests for the examples.

FetchContent can be used inside an example so the example can be copied outside the module as a starting point for a new independent application. Configuration with FetchContent is much simpler than CMake ExternalData, so the example can remain simple.

hjmjohnson · 2020-02-21T02:29:14Z

@jhlegarreta This is more than a year out of date. Can it be abandoned?

jhlegarreta · 2020-02-21T08:17:46Z

You're right Hans, it's been a while. I have not found the time to investigate more.

IMHO, having the ITKModuleTemplate a mechanism to make room for examples is useful, since we will want remotes to be as complete as possible. I do not foresee that I will have time to pick this up any time soon, so if you feel like it should be abandoned, go ahead.

If at some point someone has the cycles to tackle this, unless we delete the branch, they will be able to re-open it or to get some inspiration from this.

hjmjohnson · 2020-02-21T13:23:32Z

@dzenanz Can you please re-address your concerns with this issue? If it is an incremental improvement with known deficencies, could it be merged? If not, let's make it into an issue and close this PR.

dzenanz · 2020-02-21T13:29:44Z

It is not a simple fix. I updated the issue #34.

dzenanz · 2024-07-01T18:28:34Z

This PR was fairly close to working. Applying solution from https://github.com/InsightSoftwareConsortium/ITKMontage/blob/v0.8.2/examples/CMakeLists.txt#L4-L22, along with a general update should make it work. Jon, can you revive it?

jhlegarreta · 2024-07-01T23:28:54Z

This PR was fairly close to working. Applying solution from https://github.com/InsightSoftwareConsortium/ITKMontage/blob/v0.8.2/examples/CMakeLists.txt#L4-L22, along with a general update should make it work. Jon, can you revive it?

PR #163.

ENH: Add examples to the template.

516232a

Add a hint to foster writing examples for the contributed classes.

jhlegarreta requested review from thewtex, blowekamp, fbudin69500 and dzenanz December 23, 2018 22:22

jhlegarreta mentioned this pull request Dec 23, 2018

ENH: Transition the example to the ITK testing framework. InsightSoftwareConsortium/ITKSkullStrip#8

Merged

dzenanz reviewed Dec 24, 2018

View reviewed changes

jhlegarreta commented Dec 24, 2018

View reviewed changes

jhlegarreta mentioned this pull request Dec 24, 2018

ENH: Expose the CMake option for building the examples. KitwareMedical/ITKPhaseSymmetry#22

Merged

dzenanz mentioned this pull request Feb 21, 2020

Add examples folder content to the module template #34

Closed

dzenanz closed this Feb 21, 2020

jhlegarreta mentioned this pull request Jul 1, 2024

ENH: Add examples to the template. #163

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Add examples to the template. #46

ENH: Add examples to the template. #46

jhlegarreta commented Dec 23, 2018 •

edited

Loading

jhlegarreta commented Dec 23, 2018 •

edited

Loading

jhlegarreta commented Dec 24, 2018 •

edited

Loading

dzenanz left a comment

dzenanz Dec 24, 2018

dzenanz Dec 24, 2018

fbudin69500 Jan 2, 2019

dzenanz Jan 2, 2019

dzenanz Jul 1, 2024

jhlegarreta Jul 1, 2024

jhlegarreta Dec 24, 2018

thewtex Jan 3, 2019

jhlegarreta Jan 3, 2019

jhlegarreta Dec 24, 2018

dzenanz Dec 24, 2018

thewtex commented Jan 3, 2019

jhlegarreta commented Jan 3, 2019

thewtex commented Jan 4, 2019

hjmjohnson commented Feb 21, 2020

jhlegarreta commented Feb 21, 2020

hjmjohnson commented Feb 21, 2020

dzenanz commented Feb 21, 2020

dzenanz commented Jul 1, 2024

jhlegarreta commented Jul 1, 2024

		@@ -0,0 +1,12 @@
		cmake_minimum_required(VERSION 3.10.2)
		project({{ cookiecutter.module_name }}Examples)

ENH: Add examples to the template. #46

ENH: Add examples to the template. #46

Conversation

jhlegarreta commented Dec 23, 2018 • edited Loading

jhlegarreta commented Dec 23, 2018 • edited Loading

jhlegarreta commented Dec 24, 2018 • edited Loading

dzenanz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thewtex commented Jan 3, 2019

jhlegarreta commented Jan 3, 2019

thewtex commented Jan 4, 2019

hjmjohnson commented Feb 21, 2020

jhlegarreta commented Feb 21, 2020

hjmjohnson commented Feb 21, 2020

dzenanz commented Feb 21, 2020

dzenanz commented Jul 1, 2024

jhlegarreta commented Jul 1, 2024

jhlegarreta commented Dec 23, 2018 •

edited

Loading

jhlegarreta commented Dec 23, 2018 •

edited

Loading

jhlegarreta commented Dec 24, 2018 •

edited

Loading