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

🐛 Better error behavior for repeated calls of standardize #2266

Merged
merged 13 commits into from
Dec 10, 2024

Conversation

Zethson
Copy link
Member

@Zethson Zethson commented Dec 9, 2024

Fixes #2262

Fixes KeyError formatting

Before

KeyError: '\"cell_line\" is not a valid key, available keys are: \\'well\\', \\'sirna\\'!'"

After

KeyError: "'cell_line' is not a valid key, available keys are: 'well', 'sirna'!"

Fixes repeated calls of standardize()

curator.standardize("disease")
curator.standardize("disease")

Before

led to a traceback with 'KeyError: 'disease' is not a valid key.

After

• No unstandardized values found for 'disease'

Now hides the add_new_from_columns method from sphinx

Before this PR, the deprecated add_new_from_columns method was still listed in our documentation. It is now hidden.

@Zethson Zethson changed the title 🐛 Polish error message 🐛 Polish KeyError for invalid keys Dec 9, 2024
@Zethson Zethson changed the title 🐛 Polish KeyError for invalid keys 🐛 Better error behavior for repeated calls of standardize Dec 9, 2024
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 97.05882% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.89%. Comparing base (4c27162) to head (8e0c5f7).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
lamindb/_curate.py 94.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2266      +/-   ##
==========================================
+ Coverage   92.71%   92.89%   +0.17%     
==========================================
  Files          55       55              
  Lines        6947     7177     +230     
==========================================
+ Hits         6441     6667     +226     
- Misses        506      510       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Dec 9, 2024

@Zethson Zethson marked this pull request as ready for review December 9, 2024 14:39
@github-actions github-actions bot temporarily deployed to pull request December 9, 2024 14:43 Inactive
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
@Zethson Zethson requested a review from sunnyosun December 9, 2024 15:09
@github-actions github-actions bot temporarily deployed to pull request December 9, 2024 15:24 Inactive
@falexwolf
Copy link
Member

Ahh, main is broken because of the wetlab upgrade:

I guess I'll wait with the above until this here is merged.

@falexwolf
Copy link
Member

falexwolf commented Dec 10, 2024

The summary content is great! But due to inconsistent indendation levels, it's is hard to read through (I was confused by what "1." is and where "2." is etc.) -- can you please use headings or any other consistent way of styling things?

image

@falexwolf
Copy link
Member

Done:
image

@falexwolf
Copy link
Member

😆 -- edited at the same time!

Can now do A/B testing.

A B
image image

@Zethson
Copy link
Member Author

Zethson commented Dec 10, 2024

If you still have your source, we can go for B. It's less overwhelming and I like numbered changes. Easier to refer to.

@falexwolf
Copy link
Member

If you still have your source, we can go for B. It's less overwhelming and I like numbered changes. Easier to refer to.

Yeah, it's in the version drop down:

image

I just fixed the indenting but left everything else you had the same.

@Zethson Zethson merged commit a6c1d80 into main Dec 10, 2024
16 checks passed
@Zethson Zethson deleted the fix/standardize_output branch December 10, 2024 14:32
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.

standardize KeyError output looks bad and should likely not get thrown on rerun
3 participants