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 defaults in docstring #934

Merged
merged 3 commits into from
Jan 25, 2025
Merged

Update defaults in docstring #934

merged 3 commits into from
Jan 25, 2025

Conversation

hutch3232
Copy link
Contributor

Looks like the defaults were changed in #901 but docstring was missed.

@martindurant
Copy link
Member

martindurant commented Jan 22, 2025

It seems like the new release of botocore is incompatible with any but the newest aiobotocore, but the version constraints are not working right.

@martindurant
Copy link
Member

Perhaps should have this

--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -39,7 +39,6 @@ jobs:
         run: |
           pip install git+https://github.com/fsspec/filesystem_spec
           pip install --upgrade "aiobotocore${{ matrix.aiobotocore-version }}"
-          pip install --upgrade "botocore" --no-deps
           pip install . --no-deps

as aiobotocore should bring in exactly the right version of botocore.

@hutch3232
Copy link
Contributor Author

That change makes sense to me. I believe pips resolver does not take into account already existing package versions, so it is pretty easy to break compatibility. I think it's best to install all packages with one call to pip install (perhaps even using a requirements.in file if there are a lot).

In this case I think removing that one line makes perfect sense. I can do it if you'd like on this PR or another.

@martindurant
Copy link
Member

Yes please, let's try it here

@martindurant martindurant merged commit cdd5e07 into fsspec:main Jan 25, 2025
21 checks passed
@hutch3232 hutch3232 deleted the patch-1 branch January 25, 2025 18:04
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.

2 participants