Skip to content

[WIP] Task generation #1

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

[WIP] Task generation #1

wants to merge 14 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 18, 2020

Co-authored-by: Michal Brzus [email protected]
Co-authored-by: Chase Johnson [email protected]

Description

This PR includes the work that was completed during ohbm brainhack 2020

Status

Work in progress

Copy link

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

@benjamin-gorman

These are both listed as [WIP], which indicates that they are not ready for review or consideration.
Is that what was intended?

Additonally, the commit message of "do setup" is not sufficiently descriptive to describe the changes made in the patch. Commit messages should describe the patch.

The second commit message should also describe the work that was done to generate these files, including the command line option that was run to generate the results.

@ghost
Copy link
Author

ghost commented Jun 19, 2020

@hjmjohnson

These are both listed as [WIP], which indicates that they are not ready for review or consideration.
Is that what was intended?

Yes. The pull request, at the moment, is just to let people know where we are at / what we are working on. Work still needs to be done (e.g. tools/generate_tasks.py output needs to be tested).

Additonally, the commit message of "do setup" is not sufficiently descriptive to describe the changes made in the patch. Commit messages should describe the patch.

The commit messages are also apart of the work we need to do; the messages are not intended to be final. The commits are probably going to broken apart (e.g. a separate commit for the generation of the files) and changed as we work though it. We will be sure to describe them well.

The second commit message should also describe the work that was done to generate these files, including the command line option that was run to generate the results.

We added tools/README.md for that documentation purpose. As with the other documentation it is not very thorough at the moment.

Some of our TODOs

  • tools/generate_tasks.py rewrite
  • tools/generate_tasks.py documentation
  • tools/generate_tasks.py testing
  • tools/README.md documentation
  • python package / build checks
  • template setup

Benjamin Gorman and others added 14 commits July 8, 2020 12:59
nipype/interfaces/slicer/generate_classes.py will be used as a base
with the goal of auto generating pydra tasks
- reorder imports
- add xml_dir parameter
- add dom_from_xml function
- rename grab_xml to dom_from_binary
- add command line parameter: xml directory
- add output_dir parameter
- directory will be created if it doesn't exist
- add command line parameter: output_dir

output_dir is first so binary sources can make use of it
use .format() over f string if a keyword would be more descriptive
this removes the need to keep format in mind when generating the code
documents tools/ contents and how to use generate_tasks.py
more documentation needed

Co-authored-by: Michal Brzus <[email protected]>
Co-authored-by: Chase Johnson <[email protected]>
some quick fixes to demonstrate some of the changes needed

Co-authored-by: Michal Brzus <[email protected]>
Co-authored-by: Chase Johnson <[email protected]>
Co-authored-by: Michal Brzus <[email protected]>
Co-authored-by: Chase Johnson <[email protected]>
@hjmjohnson
Copy link

@satra Do you know of someone interested in taking over this work? We have not been able to get back at it recently.

@djarecka
Copy link
Contributor

djarecka commented Aug 6, 2020

@hjmjohnson - I will take a look!

@chasejohnson3 chasejohnson3 mentioned this pull request Dec 28, 2020
@chasejohnson3
Copy link

A new PR continuing this work was created (Pull Request #2). It was put in a new pull request because this PR#1 came from unknown repository to which I do not have access.

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.

3 participants