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

Update setup.py #35

Closed
wants to merge 1 commit into from
Closed

Update setup.py #35

wants to merge 1 commit into from

Conversation

stheid
Copy link

@stheid stheid commented May 5, 2020

https://setuptools.readthedocs.io/en/latest/setuptools.html#including-data-files
according to the official documentation of setuptools, the directory separator should be /, even on windows

https://setuptools.readthedocs.io/en/latest/setuptools.html#including-data-files
according to the official documentation of setuptools, the directory separator should be /, even on windows
@modelonrobinandersson
Copy link
Collaborator

Hi @stheid, I am unable to see anything in the documentation about the separators, is still the case? Sorry for the lack of replies.

@modelonrobinandersson modelonrobinandersson self-assigned this Jul 29, 2022
@stheid
Copy link
Author

stheid commented Jan 25, 2023

@modelonrobinandersson Yep, the documentation just moved: https://setuptools.pypa.io/en/latest/userguide/datafiles.html#package-data

See the first note
image

@modelonrobinandersson
Copy link
Collaborator

Closing this as it was resolved in 20b9ad8.
I notice that the os.path.join might be redundant as well, but I will leave this for now as we are planning on reworking this when we focus on #203

@beutlich
Copy link

Closing this as it was resolved in 20b9ad8.

I would not call it resolved: This PR was not about replacing os.path.sep concatenation by os.path.join operations, but using the POSIX slash.

@modelonrobinandersson
Copy link
Collaborator

Sorry @beutlich you are right I was a bit too quick when I closed this, I have created #206 to also replace the use of os.path.join since that would also not be proper solution according to the documentation.

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 this pull request may close these issues.

3 participants