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

Replace drop_sel with drop_var everywhere in __main__.py #505

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

dawiedotcom
Copy link
Contributor

When running the examples from the documentation using the example dataset, we received "ValueError: the first argument to .drop_sel must be a dictionary".

A similar issue was discussed in #495. A fix was proposed in #493, but some calls to .drop_sel remained.

Replacing .drop_sel with .drop_var everywhere in src/climate_indices/main.py resolved the issue.

We received "ValueError: the first argument to .drop_sel must be a
dictionary" as mentioned in monocongo#495. Replacing .drop_sel with .drop_var
resolved the issue. A similar fix was proposed in monocongo#493, but some
calls to .drop_sel remained.
@monocongo
Copy link
Owner

Thanks for this @dawiedotcom

This PR makes me (finally!) realize that we lack any tests around the main processing script, so no real way to quickly validate changes to that code.

GitHub Actions seems to be down for now so I'll wait a day or two before looking at this again, but if the builds all work as before (I don't suspect they've been broken) then I don't see any reason to not merge as-is. Thanks again @dawiedotcom for your help on this!

@monocongo monocongo merged commit 5875237 into monocongo:master Mar 29, 2023
@dawiedotcom
Copy link
Contributor Author

@monocongo Happy to contribute and thank you for the merge.

@monocongo
Copy link
Owner

monocongo commented Apr 5, 2023 via email

@xeimyname1
Copy link

xeimyname1 commented Apr 5, 2023 via email

@ben29med
Copy link

ben29med commented May 7, 2023

Pull code from the latest master branch which should contain the edits already. The release available from PyPI will soon be updated with the latest fixes, but for now, the easiest way is to just do a fresh repository clone from the master branch.

On Mon, Apr 3, 2023 at 7:28 PM xeimyname1 @.> wrote: Hi, I am new to python and I am encountering the problem stated in #495 <#495> . How do I edit main.py if using a virtual env. Thanks — Reply to this email directly, view it on GitHub <#505 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKEIHTMD2XZ46DPHD6IQMDW7NMIXANCNFSM6AAAAAAWL67S2U . You are receiving this because you were mentioned.Message ID: @.>

I have a trouble in this package (Unvalid L-moments)

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.

4 participants