Skip to content
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

Should there be remove in type hints? #106

Open
samwaseda opened this issue Jan 23, 2025 · 2 comments
Open

Should there be remove in type hints? #106

samwaseda opened this issue Jan 23, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@samwaseda
Copy link
Member

samwaseda commented Jan 23, 2025

We have extensively talked about triples like these ones:

def create_vacancy(
    structure: Atoms
) -> u(Atoms, triples=((NS.inheritsPropertiesFrom, "inputs.structure"), (EX.HasDefect, EX.Vacancy))):
    ...

def relax_structure(
    structure: Atoms
) -> u(Atoms, triples=((NS.inheritsPropertiesFrom, "inputs.structure"), (EX.HasState, EX.Relaxed))):
    ...

Now, if you think of the actual process carefully, EX.Relaxed should be revoked once create_vacancy is called. So I was wondering maybe we should have something like remove=(HasState, EX.Relaxed):

def create_vacancy(
    structure: Atoms
) -> u(
    Atoms,
    triples=((NS.inheritsPropertiesFrom, "inputs.structure"), (EX.HasDefect, EX.Vacancy)),
    remove=(EX.HasState, EX.Relaxed)
):
    ...

Now, we can also think of the case like def fill_vacancy. In this case, we can also append restrictions, i.e.:

def fill_vacancy(
    structure: Atoms
) -> u(
    Atoms,
    restrictions=((OWL.onProperty, EX.HasDefect), (OWL.someValuesFrom, EX.Vacancy)),
    triples=((NS.inheritsPropertiesFrom, "inputs.structure")),
    remove=(EX.HasDefect, EX.Vacancy)
):
    ...

... because fill_vacancy makes sense only if the structure already has a vacancy.

What do you think?

@samwaseda samwaseda added the enhancement New feature or request label Jan 23, 2025
@liamhuber
Copy link
Member

Oof. Yup.

Next, is "remove" somehow active and mandatory -- i.e. the corresponding thing must be present in restrictions/triples -- or is it just a passive guarantee that such a thing is not in the output -- i.e. if it's there we get rid of it, but it needn't be there. ...I guess the latter? And functions which will only work when it was there to get rid of to start with can include it in both places?

@samwaseda
Copy link
Member Author

The restrictions guarantee that for example HasDefect - Vacancy is there. So far it has nothing to do with remove. Now, if there is no restriction, and HasDefect - Vacancy is supposed to be removed, even though the argument does not have HasDefect - Vacancy, then it does simply nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants