Skip to content
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

organisation of files (and usability of PyPI and conda-forge distributions) #70

Open
KrisThielemans opened this issue Nov 12, 2024 · 7 comments · May be fixed by #71
Open

organisation of files (and usability of PyPI and conda-forge distributions) #70

KrisThielemans opened this issue Nov 12, 2024 · 7 comments · May be fixed by #71
Assignees

Comments

@KrisThielemans
Copy link
Contributor

We currently have 4 classes of files

  1. the yardl model
  2. the generated python/C++ files
  3. petsird_helper.* files with some additional functions
  4. example code

We anticipate that the "helpers" are going to grow. I personally wouldn't grow the examples too much in this repository, as we have https://github.com/orgs/ETSIhackers/repositories.

Currently, the "helpers" are not available for import after a pip install. It'd be easy enough to do that, but I'm not sure what the best strategy for the location of all these files are.

Suggestions?

@KrisThielemans KrisThielemans self-assigned this Nov 12, 2024
@casperdcl
Copy link
Member

Separate repo, potentially pip-installable.

@gschramm
Copy link
Collaborator

@casperdcl What is the advantage of separating them? I would argue that the helpers are strongly coupled to a specific petsird version, which is why I would include them in the same package.

@casperdcl
Copy link
Member

casperdcl commented Nov 13, 2024

The impression I have from Kris is the examples/demos/helpers are not meant to be closely tied to this repository.

I don't have an opinion either way.

Technically and separate repo/package can easily pin to specific petsird versions.

@gschramm
Copy link
Collaborator

I would consider "good and efficient helpers" an essential tool which is why I would prefer to include them.

@KrisThielemans
Copy link
Contributor Author

It does look like helpers will be necessary on top of the yardl-generated code (sadly), and see no longer see essential reason to separate repos.

As commented by @naegelejd, MRD adds generated files in the repo. I'm still reluctant to do that (but might be wrong).

I see https://github.com/ismrmrd/mrd/tree/main/python/mrd/tools I suppose we could do that as well and move the current examples. Current petsird_helpers.py doesn't really fit in tools I guess. What about

python/
   petsird/
      (generated files will end up here)
      helpers/
           geometry.py # including the rigid transformation stuff
           detection_efficiencies.py

C++ organisation is a bit different, but that'll be for later.

@casperdcl
Copy link
Member

Sure, assuming yardl doesn't complain about existing files.

@KrisThielemans
Copy link
Contributor Author

I checked. it doesn't.

casperdcl added a commit that referenced this issue Nov 13, 2024
@casperdcl casperdcl linked a pull request Nov 13, 2024 that will close this issue
casperdcl added a commit that referenced this issue Nov 13, 2024
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 a pull request may close this issue.

3 participants