search

JuliaSpace / ReferenceFrameRotations.jl

A toolbox to represent 3D rotations of coordinate frames for Julia language.
MIT License
59 stars 13 forks source link

🐛 Consideration for non-unix paths #17

Closed jakewilliami closed 3 years ago

jakewilliami commented 3 years ago

Use joinpath in includes for consideration for non-unix paths

ronisbr commented 3 years ago

Hi @jakewilliami !

Thanks for the PR. Do you really think this is necessary? The tests are passing on Windows. I have never found a single problem related to the paths.

jakewilliami commented 3 years ago

@ronisbr, Huh, good point. I don’t know why paths separated with / would work on a Windows machine—it seems safer to use joinpath, but if it’s working with forward slashes, then I guess it’s fine. Up to you!

ronisbr commented 3 years ago

If you are using Windows, can you please see if the tests pass locally? I really do not have any computer with Windows right now.

jakewilliami commented 3 years ago

Sorry, I also don’t have a windows computer! However, I can test this on a Windows computer later today (I will use one of my flatmate’s 😄). I’ll let you know what I find

ronisbr commented 3 years ago

Thanks! I will wait for the result.

jakewilliami commented 3 years ago

@ronisbr, sorry for the delay—busy week. You’re absolutely right—tested on a Windows machine, and the unix paths work.

I did some digging, and the reason for this is because I believe it uses abspath or normpath. When include is called (whose method is in base/client.jl), Base._include is called (defined in base/loading.jl). To get the path name, it calls Base._include_dependency (also in loading.jl), which calls abspath or normpath, both of which are defined in base/path.jl (here and here respectively), and both of which will correct the path separators.

Sorry about this! All the best, Jake.

ronisbr commented 3 years ago

Nice! Thanks for the info @jakewilliami!