Conversation
drreynolds
left a comment
There was a problem hiding this comment.
LGTM, aside from my question regarding naming.
There was a problem hiding this comment.
I'm not knowledgeable about setting up Python libraries, but by its name, the tools/pyproject.toml file seems to call itself tools, but the contents of that file refer to itself as both sundials-suntools (project->name) and as suntools (tool.setuptools->packages). This file then refers to it both as tools and suntools.
#. How consistent should these names be?
#. If this is only installed alongside the SUNDIALS source code then I imagine tools is sufficiently descriptive, but if this were available directly using pip then I'd think a more descriptive name would be helpful.
There was a problem hiding this comment.
Thanks for pointing out the inconsistency. I have updated both the naming and structure to make suntools more of a standalone module.
There was a problem hiding this comment.
Should we keep the tools directory? I think the original intent was we could eventually add other, non-python, tools here as well (like those in the analysis-utilities repo).
Maybe this structure would work:
sundials/
+-- tools/
| +-- suntools/
| +-- pyproject.toml
| +-- README.md
| +-- examples/
| `-- src/
There was a problem hiding this comment.
I think those could go under scripts or be folded into a submodule of suntools if they are python.
There was a problem hiding this comment.
Do we foresee having non-python tools? In which case it might be nice to keep all tools grouped under a tools directory and all python-based tools go into tools/suntools
|
This seems to be failing the CI; once it passes I'll "approve" |
|
Looks like the tarscript in the Jenkins CI is failing because of the directory changes |
No description provided.