search

MolSSI-BSE / basis_set_exchange

A repository for quantum chemistry basis sets
https://molssi-bse.github.io/basis_set_exchange/
BSD 3-Clause "New" or "Revised" License
154 stars 45 forks source link

Fix create_element_data #268

Closed lenhanpham closed 1 year ago

lenhanpham commented 1 year ago

This function is no more located in helpers but in manip

susilehtola commented 1 year ago

The question is why isn't this caught by tests?

lenhanpham commented 1 year ago

The question is why isn't this caught by tests?

Sometimes machines still can make mistakes.

Recently, the website https://www.basissetexchange.org/ has not been working at all (died????!!!!!). So I turned my mind to this package and this error happened. :-)

bennybp commented 1 year ago

Recently, the website https://www.basissetexchange.org/ has not been working at all (died????!!!!!))

Looking into it now. I think I see the problem and it will be fixed shortly

susilehtola commented 1 year ago

The question is why isn't this caught by tests?

Sometimes machines still can make mistakes.

Machines don't make mistakes. This should be included in the tests.

bennybp commented 1 year ago

It's not in the list of formats to test round tripping: https://github.com/MolSSI-BSE/basis_set_exchange/blob/9c3abc5fc71fab9b3ee4cd8fc917ace0013c3404/basis_set_exchange/tests/test_curate_roundtrip.py#L42

The test fails if I do that. That should be done, but I can merge this as-is for now.

susilehtola commented 1 year ago

It's not in the list of formats to test round tripping:

https://github.com/MolSSI-BSE/basis_set_exchange/blob/9c3abc5fc71fab9b3ee4cd8fc917ace0013c3404/basis_set_exchange/tests/test_curate_roundtrip.py#L42

The test fails if I do that. That should be done, but I can merge this as-is for now.

I think the roundtrip formats should include all formats that are supported as both input and output; at least this is the impression I got from you :stuck_out_tongue_closed_eyes: Doing round trips will at least do a better job of ensuring the library is consistent...

bennybp commented 1 year ago

It's not in the list of formats to test round tripping: https://github.com/MolSSI-BSE/basis_set_exchange/blob/9c3abc5fc71fab9b3ee4cd8fc917ace0013c3404/basis_set_exchange/tests/test_curate_roundtrip.py#L42

The test fails if I do that. That should be done, but I can merge this as-is for now.

I think the roundtrip formats should include all formats that are supported as both input and output; at least this is the impression I got from you stuck_out_tongue_closed_eyes Doing round trips will at least do a better job of ensuring the library is consistent...

The readers were originally written just to help with curation, but that evolved to be more public facing. So the tests fell behind a little bit.

In reviewing this, I've discovered a pretty serious bug in the github actions. I am going to open a PR for that soon, with some fixes. But I will merge this as-is.