enabled ufuncs for external classes

[zerothi]

This enables one to interact on external classes.
However, only if they can be converted to a sisl object.

This can be quite handy for sisl specific functions.

Fixes #747.

• Closes #
• Added tests for new/changed functions?
• Documentation for functionality in docs/
• Changes documented in changes/<pr-num>.<type>.rst

[zerothi]

@pfebrer we got #747 in! Not too hard, so why not :)

Hope it can be of use!

[pfebrer]

Cool, let's see :)

[pfebrer]

Ah, so what I had in mind was that if it takes in an ASE Atoms it also returns an ASE Atoms, not a sisl geometry 😅

[zerothi]

I think we'll just keep this, but you can do weird things now:

import sisl as si
lat = si.tile(2, 0, axis=2)
assert lat.equal(si.Lattice(2).tile(0, 2))

And, when you start playing with str you are at the mercy of the order of the registered functions.
I.e. tile exists for both Hamiltonians and geometries. So

si.tile("siesta.nc", ...)

might do a geometry, or a Hamiltonian, or a Lattice...

[zerothi]

Ah, so what I had in mind was that if it takes in an ASE Atoms it also returns an ASE Atoms, not a sisl geometry 😅

ok! I can see why that makes sense... But that requires that any functionality must be mirrored in both Geometry.new|to for it to actually do anything... Couldn't we force users to do si.rotate(ase_atoms, ...).to(ase.Atoms)? If required. I can see a benefit of sometimes only getting the sisl object back. Like the above, rather odd cases.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.89%. Comparing base (92393ad) to head (36a6ee1).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #898      +/-   ##
==========================================
+ Coverage   86.87%   86.89%   +0.01%     
==========================================
  Files         405      405              
  Lines       53396    53429      +33     
==========================================
+ Hits        46390    46427      +37     
+ Misses       7006     7002       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zerothi]

What about adding a keyword that it can grab to return sisl objects?

e.g. tile(..., sisl=True) to return a sisl object, otherwise, try the to(obj) method?

[pfebrer]

And, when you start playing with str you are at the mercy of the order of the registered functions.
I.e. tile exists for both Hamiltonians and geometries.

Interesting haha. But true that having to rely on the order of the registry is not a good thing 😅
I think we have to find a solution for this or it is not worth it to add the feature, because it will be too ambiguous. Maybe add some way of specifying what we want to do exactly? E.g.:

sisl.Geometry.rotate("siesta.nc")

and then refuse to apply any ufunc when the behavior is not well defined?

For the new -> apply_func() -> to, it is true that if the default is to convert back to the original type it might be a mess for inputs like strings. I was thinking this could be useful for missing features in other codes, and then using the sisl ufunc could be like a patch that doesn't change the code much. Maybe the default can be to output sisl stuff but have an argument to maintain the class of the input.

sisl.rotate(ase_atoms) # ---> Outputs sisl geometry
sisl.rotate(ase_atoms, keep_original=True) # ---> Outputs ASE atoms

I know that this doesn't look much better than sisl.rotate(ase_atoms).to.ase() , but I think for a user of another code it will be better. Plus, then you can generally recommend to do sisl.rotate(..., keep_original=True) regardless of the input class.

[zerothi]

And, when you start playing with str you are at the mercy of the order of the registered functions.
I.e. tile exists for both Hamiltonians and geometries.

Interesting haha. But true that having to rely on the order of the registry is not a good thing 😅 I think we have to find a solution for this or it is not worth it to add the feature, because it will be too ambiguous. Maybe add some way of specifying what we want to do exactly? E.g.:

sisl.Geometry.rotate("siesta.nc")

and then refuse to apply any ufunc when the behavior is not well defined?

Ok, I have removed the use of built-ins. Forcing users to be explicit is ok.

For the new -> apply_func() -> to, it is true that if the default is to convert back to the original type it might be a mess for inputs like strings. I was thinking this could be useful for missing features in other codes, and then using the sisl ufunc could be like a patch that doesn't change the code much. Maybe the default can be to output sisl stuff but have an argument to maintain the class of the input.

sisl.rotate(ase_atoms) # ---> Outputs sisl geometry
sisl.rotate(ase_atoms, keep_original=True) # ---> Outputs ASE atoms

I know that this doesn't look much better than sisl.rotate(ase_atoms).to.ase() , but I think for a user of another code it will be better. Plus, then you can generally recommend to do sisl.rotate(..., keep_original=True) regardless of the input class.
Agreed, I have now put in so you can do rotate(ret_sisl=True). And now it will default to return the input object type, if available.

Note, than for some generics, it will return something different. So when that is the case, it will not try to convert. E.g. center.

[zerothi]

Ok, I think this is ready! Thanks for comments!