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

allow different UID types #529

Merged
merged 16 commits into from
Jan 23, 2024
Merged

allow different UID types #529

merged 16 commits into from
Jan 23, 2024

Conversation

jmmshn
Copy link
Contributor

@jmmshn jmmshn commented Jan 18, 2024

Only allow UUIDs that can be sorted in time

Partially addresses:
#519

  • Change all the function names *uuid -> *uid
  • Added new setting for UID_TYPE
  • Added support for uuid1 and ulid via python-ulid
  • Added functions to retrieve time stamp information
  • Added tests

Copy link

codecov bot commented Jan 18, 2024

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (9bd99a8) 99.86% compared to head (923468b) 99.42%.

❗ Current head 923468b differs from pull request most recent head 82ea1b5. Consider uploading reports for the commit 82ea1b5 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #529      +/-   ##
==========================================
- Coverage   99.86%   99.42%   -0.45%     
==========================================
  Files          20       21       +1     
  Lines        1514     1556      +42     
  Branches      417      422       +5     
==========================================
+ Hits         1512     1547      +35     
- Misses          2        9       +7     
Files Coverage Δ
src/jobflow/core/flow.py 100.00% <100.00%> (ø)
src/jobflow/core/job.py 100.00% <100.00%> (ø)
src/jobflow/core/store.py 100.00% <100.00%> (ø)
src/jobflow/settings.py 100.00% <100.00%> (ø)
src/jobflow/utils/__init__.py 100.00% <100.00%> (ø)
src/jobflow/utils/uuid.py 100.00% <100.00%> (ø)
src/jobflow/utils/uid.py 81.57% <81.57%> (ø)

@jmmshn jmmshn changed the title allow different uuids (but only implemented uuid1) allow different UID types Jan 19, 2024
@jmmshn jmmshn mentioned this pull request Jan 19, 2024
@gpetretto
Copy link
Contributor

Thanks for implementing the change and making uuid4 the default.
It would be convenient for me (and maybe for others), not to have an abrupt change in the exposed functions as I am using the suuid function to generate a string uuid in a few places. It is not such a big deal, but would it be fine to keep the suuid function in the uuid module with a deprecation warning?

@jmmshn
Copy link
Contributor Author

jmmshn commented Jan 20, 2024

Yeah that makes sense.

@jmmshn
Copy link
Contributor Author

jmmshn commented Jan 20, 2024

BTW does anyone know what the s in suuid stands for?

@utf
Copy link
Member

utf commented Jan 22, 2024

Thanks @jmmshn.

Is it possible to make ulid an optional dependency and only import it if the user has asked for it in their settings file. The code should also throw an appropriate error explaining which python package is needed to use ulid if it is not installed.

The s in suuid stands for string as we are encoding the UUIDs as strings not bytes.

@utf
Copy link
Member

utf commented Jan 23, 2024

Thanks @jmmshn

@utf utf merged commit 8e5a3fe into materialsproject:main Jan 23, 2024
7 checks passed
@jmmshn
Copy link
Contributor Author

jmmshn commented Jan 23, 2024

BTW, can we get a release with this, then I can finish the atomate2 PR.

@utf utf added the feature A new feature being added label Jan 25, 2024
@utf
Copy link
Member

utf commented Jan 25, 2024

Done (v0.1.17)

@utf utf added the enhancement New feature or request label Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature A new feature being added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants