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

Fix PTH118 os.path.join() should be replaced by Path with / operator #1743

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

brianhelba
Copy link
Collaborator

The actual problem here is inappropriate use of os.path.join for joining components of a URL (which should always use /, not an OS-dependent delimiter).

Joining the components with f-strings matches the way other parts of the code create URLs.

Future work could use some URL library to manipulate URLs, but that's out of scope of this fix.

@brianhelba brianhelba requested a review from jjnesbitt November 9, 2023 00:20
@brianhelba brianhelba merged commit f31367b into master Nov 9, 2023
9 checks passed
@brianhelba brianhelba deleted the url-slash branch November 9, 2023 17:22
@dandibot
Copy link
Member

dandibot commented Nov 9, 2023

🚀 PR was released in v0.3.61 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Nov 9, 2023
brianhelba added a commit that referenced this pull request Dec 13, 2023
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Dec 14, 2023
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Dec 14, 2023
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Dec 14, 2023
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Dec 14, 2023
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Dec 14, 2023
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Dec 15, 2023
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Dec 15, 2023
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Jan 3, 2024
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Feb 9, 2024
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Feb 9, 2024
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Feb 9, 2024
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
brianhelba added a commit that referenced this pull request Feb 9, 2024
Following #1743, this removes the remaining uses of the filesystem APIs
`os.path` and `pathlib` (which are platform-dependent and contain methods for
local filesystem manipulation) to process URLs.

The `zarr_checksum` package still has this confusion in its `ZarrArchiveFile`
class, but attempts to refactor some of that code failed due to other issues
with it. So, the `ZarrArchiveFile.path` field is just cast to a `str` when
interfacing with local application code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants