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

Add fpoffline package #52

Closed
dkirkby opened this issue Jun 10, 2022 · 9 comments
Closed

Add fpoffline package #52

dkirkby opened this issue Jun 10, 2022 · 9 comments
Assignees

Comments

@dkirkby
Copy link
Member

dkirkby commented Jun 10, 2022

I would like to include desihub/fpoffline in the list of DESI packages used by desiconda.

Shall I make a PR with this added to scripts/bootstrap-desi.sh or is there a different procedure?

@sbailey
Copy link
Contributor

sbailey commented Jun 11, 2022

I ran desiInstall fpoffline main as the desi user to install it as a module; please try module load fpoffline and try using it to see if it is configured correctly (it passes basic ability to load the module, but other that that I don't know what to try).

If that works, you can then add it to scripts/bootstrap-desi.sh to be included by default in future desiconda releases. When you make tags, we can also install those.

@dkirkby
Copy link
Member Author

dkirkby commented Jun 13, 2022

Thanks. I tested that I can import and run fpoffline modules, but it looks like the executable script generated by pip install . (via setup.py entry_points) is not created or, at least, not visible via $PATH.

Looking desimeter, it seems that executable scripts need to be under a top-level bin/ folder? Is there a way to accomplish that and still use the setup.py entry_points machinery?

@sbailey
Copy link
Contributor

sbailey commented Jun 13, 2022

I think desiInstall would trigger entry_points for installations of tags, but main and other branches use "in place installations" of a bare git checkout adding bin/ to $PATH and either py/ or the top level directory to $PYTHONPATH. This enables "git pull" to update the installation without further steps, and allows developers to do the equivalent for their own checkouts so that edits are immediately visible without having to go through an installation step.

FTR: I'm also aware of "python setup.py develop" as something in-between, but that gets confused when the version number changes, while the in-place installations are robust to that.

i.e. I recognize that entry_points is a common python practice, but we discourage using it and encourage putting executable scripts in bin/, explicitly wrapping functions from the module, i.e. doing ahead of time what entry_points otherwise does at installation time.

@dkirkby
Copy link
Member Author

dkirkby commented Jun 13, 2022

I should make a tag anyway and it sounds like this would fix the problem?

@sbailey
Copy link
Contributor

sbailey commented Jun 13, 2022

it would fix the problem (I think) for the purposes of the tagged installation, though the main installation would remain without the executables. Please ping me when the tag is ready and we'll try that and then iterate if the tag installation is also broken.

@weaverba137
Copy link
Member

@sbailey, @dkirkby, it sounds like the solution was to install fpoffline with desiInstall. Is that fully implemented?

@weaverba137
Copy link
Member

Since it's been a year, I'll ask again: @sbailey, @dkirkby, it sounds like the solution was to install fpoffline with desiInstall. Is that fully implemented?

@sbailey
Copy link
Contributor

sbailey commented Aug 13, 2024

We never got far enough to include fpoffline in scripts/bootstrap-desi.sh, and it looks like fpoffline isn't updated to use a bin/ directory with script wrappers instead of entry_points, and it doesn't have any tags so we haven't tested that either. I think that is a small refactor, but given how old this ticket is I suspect it hasn't been high priority.

@dkirkby if this is something you still want, please update fpoffline to have scripts in a bin/ directory (to support desiInstall installations of main) and/or make a fpoffline tag that we can test for a tag installation.

@dkirkby
Copy link
Member Author

dkirkby commented Aug 13, 2024

Thanks for following up on this @sbailey and @weaverba137. Now that I understand how desiInstall works a bit better, I prefer to stick with the python-standard entry_points mechanism since that is probably more convenient for most users of this package. I will close this issue now.

@dkirkby dkirkby closed this as completed Aug 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

No branches or pull requests

3 participants