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

Print warning when a property is guessed #2222

Open
kain88-de opened this issue Mar 16, 2019 · 4 comments

Comments

@kain88-de
Copy link
Member

commented Mar 16, 2019

See: https://twitter.com/sperezconesa/status/1106556146427543552

@MDAnalysis I think it would be nice if MDAnalysis gave a warning message if masses are being guessed and not read from files (this is general for any guess property). It can give problems to guess stuff and not warn the user. Keep up the good work!! 😀

@sperezconesa

This comment has been minimized.

Copy link

commented Mar 19, 2019

Hello,
In addition to what I said in the tweet. I saw the other day a video of prof. Woods about some principles he is using in his latest software project. I thought many of them were good in general for software. One of them was something like: never guess anything unless the user specifically asks it.

https://www.youtube.com/watch?v=w1d1xtbGhHc&t=1434s

@jbarnoud

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

It just did bite me. The PDB writer guess the atom elements. Not only did it get one of them wrong, but there is no way of provided the right elements.

@orbeckst

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

Where do we guess automatically?

  • PDB masses
  • PDB bonds

Anywhere else? It would be good to have such a list. We can then open issues for specific cases.

In general I agree that I'd rather fail than work with dubious data. What I then like in those cases is a suggestion how to fix the failure or how to explicitly take the risk.

@jbarnoud

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2019

orbeckst added a commit that referenced this issue Aug 2, 2019

Parsing negative values in ATOMIC_NUMBER records of AMBER topologies (#…
…2307)

* Negative values in the ATOMIC_NUMBER records are now read by guessing the atom element using `guessers.guess_atom_element`.
* Switch for dummy atoms ("DUMMY" as element)
* DUMMY atm guess + tests
* Changelog update
* Tests for new TOPParser warnings
* fixes #2306
* element guesses  in the Amber parser now output user warnings (for #2222)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.