Skip to content

Conversation

@jadball
Copy link
Contributor

@jadball jadball commented Feb 25, 2025

No description provided.

@jadball
Copy link
Contributor Author

jadball commented Jul 2, 2025

Bump, @jonwright happy to merge this in?

@jonwright
Copy link
Member

Sorry - I don't remember looking at it? The CI is red and I don't know why - was it a broken dependency?
I didn't write this xfab code and I don't read regex language, so with that context in mind:

  • The first one looks like it is removing whitespace. Could also be "".join(symbol.split()) but the regex is probably the same.

  • I guess the second one is looking for a string that matches something in atomlib.formfactor.keys(). This sounds like:

def xfab_atom_site_type_symbol( name, keys = tuple( xfab.atomlist.formfactor.keys()) ):
   """ Find the matching atom in our data table """
   if name in keys: # Allows us to add ionic form factors to atomlib without changing this code.
        return name
   if name[:2] in keys: 
        if len( name )>2 : logging.warning( "Replacing " + name, + " with " + name[:2] )
        return name[:2]
   if name[0] in keys:
        if len( name )>1 : logging.warning( "Replacing " + name, + " with " + name[0] )
        return name[0]
   raise Exception( name + " is not a known atomtype for xfab")

In theory, we could also fix this by adding the ionic form factors to atomlib. There are some recent data tabulated here: https://doi.org/10.1107/S2053273323010550 . Probably just means reformatting the tables from that paper, but I haven't checked if it is using the same gaussian expansion as the one in xfab.

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