Skip to content

Conversation

@vaitkus
Copy link
Collaborator

@vaitkus vaitkus commented Jul 30, 2025

See issue COMCIFS/dREL#20 for more details.

Copy link
Contributor

@jamesrhester jamesrhester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose explicitly setting 'missing' (the default value anyway) is OK here to simplify transformation of the dREL - but the value will be 'missing' if dREL fails to set it to something else.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Jul 31, 2025

On second though, it might be better to change the ? to unspecified, that is, to "figure it out yourself"?

Also, this is the first dREL snippet which I have seen that modifies the definition (units) for each entry in the loop, i.e. different values of the same data item in the same CIF file will get different units. I guess this is allowed?

@jamesrhester
Copy link
Contributor

First time I've seen it as well. But that is the point of these Definition methods, they are for situations where the values are row-dependent. The default conversion matrix to Cartesian coordinates is also cell-dependent, i.e. depends on _structure.id, so is row-dependent if you were to tabulate cells against _structure.id.

The "unknown" keyword is not supported by dREL and the units
are better described as unspecified than unknown.
@vaitkus vaitkus force-pushed the fix-unknown-values-in-drel-code branch from 1a5489b to 2335944 Compare August 1, 2025 06:51
@vaitkus
Copy link
Collaborator Author

vaitkus commented Aug 1, 2025

I changed the unknown values to "unspecified" since it better describes the situation.

First time I've seen it as well. But that is the point of these Definition methods, they are for situations where the values are row-dependent. The default conversion matrix to Cartesian coordinates is also cell-dependent, i.e. depends on _structure.id, so is row-dependent if you were to tabulate cells against _structure.id.

I have encountered 1) dREL that calculates row dependent values and 2) dREL that modifies the definition of an item from a Set category based on the values of other items (most often measurement units are modified), but not the combination of 1) and 2) as in this case. I was simply surprised, that's all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants