-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add convenience util func for generating robocrys descriptions #1076
Conversation
robocrys is an extra import in emmet-core
@esoteric-ephemera, can you take a look at this when you have a chance? Not sure if this still applies with the recent robocrys updates |
Yes! Have some specific thoughts right now having run this a bunch: there are a few common exceptions that get thrown by robocrys that I don't think we're handling:
I think we can maybe treat the first one by adjusting The Voronoi exception is from pymatgen, might be too buried to adjust through robocrys kwargs but I'll look into that Will also add some defaults logic - we probably want some additions to the builder too to reduce load time in mineral matcher |
All sounds good From the builder side the plan was just to pass in an already instantiated mineral matcher to the Just instance like |
@tsmathis sorry for the delay, adding some extra logic for handling some common exceptions - think this is in good shape |
nice, thanks! |
one thing, since robocrys is optional for emmet how do you want to handle the |
46fa664
to
c852d4d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1076 +/- ##
===========================================
+ Coverage 72.66% 90.18% +17.52%
===========================================
Files 77 147 +70
Lines 5015 14174 +9159
===========================================
+ Hits 3644 12783 +9139
- Misses 1371 1391 +20 ☔ View full report in Codecov by Sentry. |
Since we're using the legacy type hints, I think your solution (just removing the type hint for |
@esoteric-ephemera, any extra additions when you get a chance.
Likely need some flexibility with kwargs, or any defaults you think are sane.