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

Windows str.replace not implemented properly #66

Open
AndyEveritt opened this issue Oct 21, 2020 · 3 comments
Open

Windows str.replace not implemented properly #66

AndyEveritt opened this issue Oct 21, 2020 · 3 comments

Comments

@AndyEveritt
Copy link

You often have a line such as this to fix windows file paths:

# Fix windows path if needed
file_path_on_target_machine.replace('\\', '/')

This does not actually edit the variable but instead returns a new variable as described here (https://docs.python.org/2/library/string.html#string.replace)

I suggest your code should be updated to file_path_on_target_machine = file_path_on_target_machine.replace('\\', '/')

This will remove the need for users to do this themselves before the saleae methods a file_path if using Windows

@AndyEveritt
Copy link
Author

Although, for some of the methods it does not appear to be an issue whether the file path has \\ or / so it may be a null issue. If this is the case then I think the code should be removed since it is not needed or doing anything

@ppannuto
Copy link
Owner

Hah! Nice catch.. I don't actually have any Windows machines, so all of the windows code in this library is from other folks. Given that it's not actually doing anything and no one is complaining about that, probably best to just drop them. Happy to take a PR -- thanks!

@AndyEveritt
Copy link
Author

I'm doing a project with the saleae currently so should have a good chance to confirm whether this is actually an issue or not. Either way I can certainly make a PR once I know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants