Skip to content

MAINT chmod ±x #21234

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 5, 2021
Merged

MAINT chmod ±x #21234

merged 1 commit into from
Oct 5, 2021

Conversation

DimitriPapadopoulos
Copy link
Contributor

What does this implement/fix? Explain your changes.

Run chmod +x on some files that are supposed to be executable, this is consistent with other similar files:

build_tools/github/build_minimal_windows_image.sh
build_tools/github/build_source.sh
build_tools/github/build_wheels.sh
build_tools/github/check_build_trigger.sh
build_tools/github/repair_windows_wheels.sh
build_tools/github/test_source.sh
build_tools/github/test_wheels.sh
build_tools/github/test_windows_wheels.sh
build_tools/github/upload_anaconda.sh
build_tools/travis/install.sh
build_tools/travis/install_wheels.sh
build_tools/travis/script.sh
build_tools/travis/test_wheels.sh

Run chmod -x on some files that are not supposed to be executable, again for consistency with other similar files:

sklearn/neighbors/_binary_tree.pxi
sklearn/neighbors/_dist_metrics.pyx

Any other comments?

What about examples? They start with a shebang, but they're just examples. Some of them are executable, others are not. Typically, should the following be executable or not?

./doc/tutorial/machine_learning_map/*.py
./examples/*/*.py

Finally I have kept this file non-executable, because it is "dangerous":

.binder/postBuild

scikit-learn/.binder/postBuild

Lines 10 to 12 in d152b1e

echo "This script was written for repo2docker and is supposed to run inside a docker container."
echo "Exiting because this script can delete data if run outside of a docker container."
exit 1

@DimitriPapadopoulos DimitriPapadopoulos changed the title chmod ±x MAINT chmod ±x Oct 4, 2021
rth
rth approved these changes Oct 5, 2021
View reviewed changes
Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable, thanks!

What about examples? They start with a shebang, but they're just examples. Some of them are executable, others are not. Typically, should the following be executable or not?

I think it doesn't matter, people should execute them by running python example.py in any case.

@DimitriPapadopoulos
Copy link
Contributor Author

I think it doesn't matter, people should execute them by running python example.py in any case.

Then remove the shebang.

jjerphan
jjerphan approved these changes Oct 5, 2021
View reviewed changes
Copy link
Member

@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @DimitriPapadopoulos.

@rth
Copy link
Member

rth commented Oct 5, 2021

Then remove the shebang.

I would actually +1 for that in examples. For instance,

#!/usr/bin/python

make no sense whatsoever to me. Unless there are some users that double click on examples on Linux or Mac and it works? Could you please open a separate issue about this?

@rth rth merged commit d3429c1 into scikit-learn:main Oct 5, 2021
@DimitriPapadopoulos DimitriPapadopoulos deleted the chmod branch October 5, 2021 21:08
@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request Oct 5, 2021
Merged
@DimitriPapadopoulos DimitriPapadopoulos restored the chmod branch October 5, 2021 21:52
@DimitriPapadopoulos DimitriPapadopoulos deleted the chmod branch October 5, 2021 21:52
@glemaitre glemaitre mentioned this pull request Oct 23, 2021
Merged
10 tasks
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Oct 23, 2021
@DimitriPapadopoulos @glemaitre
MAINT Fix chmod ±x in .sh and .pyx files (scikit-learn#21234)
e485ab4
glemaitre pushed a commit that referenced this pull request Oct 25, 2021
@DimitriPapadopoulos @glemaitre
MAINT Fix chmod ±x in .sh and .pyx files (#21234)
e65ba33
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
@DimitriPapadopoulos @samronsin
MAINT Fix chmod ±x in .sh and .pyx files (scikit-learn#21234)
b46cf23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rth rth rth approved these changes

@jjerphan jjerphan jjerphan approved these changes

Assignees
No one assigned
Labels
cython module:neighbors
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DimitriPapadopoulos @rth @jjerphan