proposal for setup.py revision for issue #1729

[bmorris3]

Background

This pull request explores options for updating setup.py. At present, there are several ways for a user to install jdaviz, which each have slightly different behaviors, especially with regards to installing the voila template. Important background: the "jdaviz voila template" is a series of files located within share/. I'll refer to copying or symlinking the jdaviz voila template files into a location where voila expects to find them as "installing" the custom template.

The options are:

1. Install with python setup.py install or pip install .. Setuptools will copy the data files being passed through the setup(data_files=data_files) keyword argument into the correct locations (background). The copy of the template files is static.

2. Install with python setup.py develop, which has been deprecated. In that case, our custom develop command class in setup.py gets triggered, which tries to make a symlink, or if not, a copy of the directories.

3. You can also do pip install -e ., which does not use the data_files kwarg given to setuptools, nor does it use the custom develop command class. No template files get copied when you do the developer install. This seems to arise from subtleties in the differences between python setup.py develop and pip install -e ., which are not identical implementations.

Since most users are pip installing jdaviz, they get the template files successfully via data_files. I'm wondering if devs haven't hit a problem with this workflow because at some point, we all once pip installed jdaviz, which gives us the template, before calling pip install -e . for development, which does not give you the template.

Description

This PR does a few things:

1. Moves the template management operations out of the methods on the custom subclass of develop, and into self-contained functions in setup.py.

2. Introduces a custom install subclass that uses exactly the same template symlink/copy operation as develop. <- update: It turns out that moving to symlink defaults can fail, so I'm removing this feature and falling back to relying on the data_files kwarg to install a copy of the files.

3. Adds voila to pyproject.toml so that we can use the voila template path finding functions within our setup.py file

(1) implements functions that users could intentionally call to symlink/copy their custom template files without re-installing the custom files' host package (jdaviz in this case). I'm implementing these functions with the expectation that they might belong in an upstream PR to voila. Any thoughts? (Sidebar: at first, I also wanted users to be able to call these functions at will in case they/we edit the templates, but it turns out you can't import functions from a setup.py file 🤷🏻‍♂️. Oh well.) With (2), I was trying to reduce some of the uniqueness of install vs develop update: but I have since had to ditch that effort and keep them different. (3) is required for now, but if (1) becomes a PR to voila, then (3) can be removed.

I hope this is useful for @rosteen, satisfies @kecnry's hesitance to introduce dependencies, and successfully meets the criteria to close #1729. I'm marking no-changelog-entry-needed because users shouldn't notice a difference.

Fixes #1729

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.89%. Comparing base (36d4c9f) to head (51a6655).
Report is 1623 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1792      +/-   ##
==========================================
+ Coverage   87.82%   87.89%   +0.06%     
==========================================
  Files          95       95              
  Lines       10178    10185       +7     
==========================================
+ Hits         8939     8952      +13     
+ Misses       1239     1233       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pllim]

I was expecting that we can get rid of cmdclass={'develop': DevelopCmd} altogether but I don't see that happening in the diff.

[pllim]

👎 on this because it introduces voila as build-time dependency even though it is only needed for the standalone app part.

But you say we don't need this if some stuff is moved upstream? If that is the case, should we move the stuff upstream and not deal with this at all?

[kecnry]

Since most users are pip installing jdaviz, they get the template files successfully via data_files. I'm wondering if devs haven't hit a problem with this workflow because at some point, we all once pip installed jdaviz, which gives us the template, before calling pip install -e . for development, which does not give you the template.

I think this is the case for myself, and really just has to be done once on the machine not for each environment (assuming the voila templates don't change, which they don't often). So if for some reason we decide not to go forward with this, we could alternatively put this somewhere in the dev docs instead.

[duytnguyendtn]

Something that just struck me, which might be relevant here. We should make sure we aren't running in circles here: #925

We removed symlinking as part of our build process because it requires administrative rights on Windows.

[rosteen]

@bmorris3 Can we close this, given that we no longer use Voila?

[bmorris3]

This is not necessary anymore.