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

migrate to >=py311 enum.StrEnum usage #1144

Merged
merged 2 commits into from
Oct 8, 2024
Merged

migrate to >=py311 enum.StrEnum usage #1144

merged 2 commits into from
Oct 8, 2024

Conversation

bjlittle
Copy link
Owner

@bjlittle bjlittle commented Oct 8, 2024

🚀 Pull Request

Description

After adopting Scientific Python SPEC-0 and dropping support of py310, we're now free to adopt enum.StrEnum and enum.auto and thus purge some tech debt.


@github-actions github-actions bot added the type: testing Auto-labelled label Oct 8, 2024
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.28%. Comparing base (9ca4121) to head (4bf1317).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1144      +/-   ##
==========================================
- Coverage   91.29%   91.28%   -0.01%     
==========================================
  Files          58       58              
  Lines        2941     2939       -2     
==========================================
- Hits         2685     2683       -2     
  Misses        256      256              

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

@bjlittle bjlittle merged commit b280d60 into main Oct 8, 2024
21 checks passed
@bjlittle bjlittle deleted the py311-strenum branch October 8, 2024 15:48
@trexfeathers
Copy link
Collaborator

Are you sure about auto()? With explicit strings we still get the built-in validation and documentation of Enum, while allowing users to type strings in their own code, which I imagine would be easier.

@bjlittle
Copy link
Owner Author

bjlittle commented Oct 10, 2024

@trexfeathers Users can still type strings as before, so there's no difference in user experience or behaviour.

When auto() is used for a StrEnum it's giving us the lower case string value of the enumeration name for free ... the only difference is in the rendered documentation where the value of an enumeration isn't shown i.e., this ...

image

vs this ...

image

TBH on reflection I prefer the former, so if you wanted to push a PR dropping the use of auto for the explicit lower case literal, that would be welcomed 😉

@trexfeathers
Copy link
Collaborator

Ohhhh now I can't decide 😖

it's giving us the lower case string value of the enumeration name for free

That's good!

the rendered documentation where the value of an enumeration isn't shown

That's bad!

TBH on reflection I prefer the former

Let me know if you're just being nice. If you really want it I will write it!

@bjlittle
Copy link
Owner Author

@trexfeathers Let's go with reverting the use to auto and replace with lower-case literals. This should improve the documentation, if you're still game for that?

@trexfeathers
Copy link
Collaborator

#1152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: testing Auto-labelled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants