Opened 4 years ago
Closed 2 years ago
#27171 closed defect (fixed)
Move file src/bin/sagemaxima.lisp, used by sage at import time, to live inside the package
Reported by:  Erik Bray  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  refactoring  Keywords:  
Cc:  François Bissey, Timo Kaufmann, John Palmieri, Frédéric Chapoton  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  da05b3c (Commits, GitHub, GitLab)  Commit:  da05b3c2df4f2c98cef3ba392187505abd763dd4 
Dependencies:  #21559  Stopgaps: 
Description (last modified by )
Nonbinary files that are part of the sage sources and needed by the sage package at import time should be installed in the package, using package_data
in setup.py
. See e.g. https://trac.sagemath.org/ticket/27040#comment:48
Change History (15)
comment:1 Changed 4 years ago by
Cc:  François Bissey added 

comment:2 Changed 4 years ago by
Cc:  Timo Kaufmann added 

comment:4 followup: 6 Changed 4 years ago by
Replying to embray:
If it's needed by the sage Python package to work then it should just be installed inside the package (e.g. as
package_data
insetup.py
)
Why should it be installed like that?
I'm not against moving the specific file sagemaxima.lisp
, but you seem to imply a more general rule here that Python packages should never access files outside of their own package (or something like that?).
comment:5 Changed 4 years ago by
Description:  modified (diff) 

comment:6 Changed 4 years ago by
Replying to jdemeyer:
Replying to embray:
If it's needed by the sage Python package to work then it should just be installed inside the package (e.g. as
package_data
insetup.py
)Why should it be installed like that?
I'm not against moving the specific file
sagemaxima.lisp
, but you seem to imply a more general rule here that Python packages should never access files outside of their own package (or something like that?).
That's definitely not what I'm saying, though I can see where you get the implication.
This is definitely not a rule in case of files that are not part of Sage, which may come from other packages or be overridden in some way by downstream packagers or by users. In those cases we want an option, with some reasonable default, for where to find that file.
In this case, if I understand correctly, it's just Maxima startup code very specific to Sage's Maxima interface. So there's no reason for it to live anywhere else outside the sage package, and that also makes the question of "where to find it" much simpler, because it's just relative to the package's installation path. And it certainly doesn't belong in any bin/
.
Did the same with sage.gaprc
in sage.libs.gap
.
comment:7 Changed 4 years ago by
Milestone:  sage8.7 → sagepending 

Removing most of the rest of my open tickets out of the 8.7 milestone, which should be closed.
comment:8 Changed 2 years ago by
Milestone:  sagepending → sage9.2 

Summary:  Move files used by sage at import time to live inside the package → Move file src/bin/sagemaxima.lisp, used by sage at import time, to live inside the package 
comment:9 Changed 2 years ago by
Dependencies:  → #21559 

comment:10 Changed 2 years ago by
Authors:  → Matthias Koeppe 

comment:11 Changed 2 years ago by
Branch:  → u/mkoeppe/move_file_src_bin_sage_maxima_lisp__used_by_sage_at_import_time__to_live_inside_the_package 

comment:12 Changed 2 years ago by
Cc:  John Palmieri Frédéric Chapoton added 

Commit:  → da05b3c2df4f2c98cef3ba392187505abd763dd4 
Status:  new → needs_review 
1 commit on top of #21559.
Last 10 new commits:
4a3d36e  Move 'sage app' back to src/bin/sage

3a0193c  src/bin/sage: Remove handling of 'sage axiom'

6b04075  Merge branch 't/29111/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution' into t/21559/changesrcbininstallation

9c7116b  src/bin/sagelistoptional, sagelistexperimental, sageliststandard: Remove deprecated scripts

831cc09  Merge branch 't/29920/remove_deprecated_scripts_sage_list_optional__sage_list_experimental__sage_list_standard' into t/21559/changesrcbininstallation

a56dc35  Merge tag '9.2.beta1' into t/29702/public/move_all_code_from_src_setup_py__src_fpickle_setup_py_to_sage_setup

e3eca85  Merge branch 'public/move_all_code_from_src_setup_py__src_fpickle_setup_py_to_sage_setup' of git://trac.sagemath.org/sage into t/21559/changesrcbininstallation

7d29141  src/setup.py: Do not install removed script sagersyncdist

39cb52a  Merge branch 't/21559/changesrcbininstallation' into t/27171/move_file_src_bin_sage_maxima_lisp__used_by_sage_at_import_time__to_live_inside_the_package

da05b3c  src/bin/sagemaxima.lisp: Move inside package, install as package_data

comment:13 Changed 2 years ago by
Reviewers:  → François Bissey 

Status:  needs_review → positive_review 
This is sorely needed. I follows the model of other packages for which it was already done. I want this in. LGTM.
comment:15 Changed 2 years ago by
Branch:  u/mkoeppe/move_file_src_bin_sage_maxima_lisp__used_by_sage_at_import_time__to_live_inside_the_package → da05b3c2df4f2c98cef3ba392187505abd763dd4 

Resolution:  → fixed 
Status:  positive_review → closed 
What do you mean with "live inside the package"?