Improve grain boundary docstr + check on query fields #992
Conversation
| gb_energy: tuple[float, float] | None = None, | ||
| pretty_formula: str | None = None, | ||
| rotation_axis: list[str] | None = None, | ||
| rotation_axis: tuple[str | float, str | float, str | float] | None = None, |
There was a problem hiding this comment.
Does this really work as a union?
and also why is the union str | float? The data types in the db are all int:
db.grain_boundaries.aggregate([
{
$project: {
_id: 0,
init_axis: "$initial_structure.rotation_axis",
final_axis: "$final_structure.rotation_axis",
},
},
{
$facet: {
init_axis: [
{ $unwind: "$init_axis" },
{ $group: { _id: { $type: "$init_axis" }, count: { $sum: 1 } } },
],
final_axis: [
{ $unwind: "$final_axis" },
{ $group: { _id: { $type: "$final_axis" }, count: { $sum: 1 } } },
],
},
},
]);
-----
[
{
init_axis: [ { _id: 'int', count: 1008 } ],
final_axis: [ { _id: 'int', count: 1009 } ]
}
]There was a problem hiding this comment.
obviously if this all "just works" since later on everything is cast to str and joined as a comma separated string then it's fine
There was a problem hiding this comment.
The str is a holdover from before, the float is wrong, I'll change that to int. To make the API request, it just needs to format into a str like you said, so it's OK
| ) | ||
|
|
||
| if rotation_axis: | ||
| if len(rotation_axis) != 3: |
There was a problem hiding this comment.
there are also instances of rotation_axis = [ 0, 0, 0, 1 ]
There was a problem hiding this comment.
You're right - changed this overall just to use tuple[ int * 3] or tuple[int * 4] as the type hint and for the value check
|
@esoteric-ephemera Is this ok to merge or does it need additional changes? |
|
@tschaume ready to merge! |
3 participants
Related to #991, the type hinting and docstr for
rotation_axisinGrainBoundaryResterwere misleading