-
Notifications
You must be signed in to change notification settings - Fork 43
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
More robust name validation #703
base: main
Are you sure you want to change the base?
Conversation
3a545d3
to
a42d698
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #703 +/- ##
==========================================
+ Coverage 91.89% 92.03% +0.13%
==========================================
Files 45 46 +1
Lines 6919 6977 +58
==========================================
+ Hits 6358 6421 +63
+ Misses 561 556 -5
|
Excellent PR @aeisenbarth, thank you! I performed my code review and applied directly the code changes. I list them here:
|
I ask you please to give a double check to my code changes, and if you agree with them (or after your edits), let's merge 😊 |
The explanation in the Discussions on how to be able to read datasets with naming problems is great! One minor todo:
|
Thanks, the changes are good.
The exception is not raised in a single place. These exceptions would need to be changed: Probably we would rather refactor the code or create a wrapper function that adds the link to a raised exception, and call that function in place of |
We just discussed following ideas in the meeting:
Any opinions, @LucaMarconato, @giovp ? |
Closes #624
.
(now allowing_-.
and alphanumeric, which includes0-9a-zA-Z
but also other Unicode likeɑ
and²
).
,..
__
abc
,Abc
(only one of them allowed, no matter which case)obs
,obsm
,obsp
,var
,varm
,varp
,uns
).obs
andvar
dataframes,_index
is forbidden.