Document NoDataError better and ensure it's properly applied everywhere

[IAlibay]

As per #3898 and #3837 before it, the current interpretation of NoDataError seems to be that it exists within the realm of TopologyAttrs. If this is the interpretation we are going for, we should do the following:

1. Clarify the documentation for this: "Raised when empty input is not allowed or required data are missing." is probably too non-specific.
2. Make sure that the codebase reflects this. In cases where NoDataError is used for zero-width arrays or empty AtomGroups, then we should make sure we raise a ValueError. I'm not sure how many cases of this there are, but MemoryReader.timeseries doesn't raise NoDataError on empty AtomGroup, while DCDReader.timeseries does #3898 does point out that it's probably happening in quite a few places.

[Nazi-pikachu]

Hi @IAlibay I am new to open source and excited to contribute to this library. Can i work on this issue?

[hmacdope]

Hi @Nazi-pikachu. MDAnalysis has a diverse range of contributors and values a respectful, tolerant, and open community. We would love to have you contribute as a member of our community, however unfortunately your name "nazi-pikachu" violates our Code of Conduct, specifically Section 5 which says:

"""
Be careful in the words that you choose. Be kind to others. Do not insult or put down other community members. Harassment and other exclusionary behavior are not acceptable. This includes, but is not limited to:

• threats or violent language directed against another person
• discriminatory jokes and language

"""
We would allow you to contribute under another github username, but cannot accept your contributions under your current moniker.

[Nazi-pikachu]

Hi @hmacdope I regret that my github username is a violation of code of conduct. Thank for pointing it out , I was not aware of it. Now i will look for altenative to creating a seperate account as all my existing contribution is from this account, any suggestion from you will be helpful.

[hmacdope]

I would suggest either using a new account or changing your name, whichever you prefer.

[chandra20500]

Hi, I would like to work on this Issue. it would be great could you please assign this issue to me

[Adarshsantrapatro]

hii i would like to work on this issue can you assign me this

[hmacdope]

Hi @chandra20500 and @Adarshsantrapatro, we do not assign people issues. Just make a PR and we will focus on the first PR made on that issue.

[rajeck1234]

hlw i want to contribute on this issue

[RMeli]

Hi @utkarshraj779 , as you can read above you can open a PR solving this issue. We will look at the first PR that comes through solving a particular issue.

[ammarfitwalla]

Hi , I am new to contributing to open source , can I have this issue resolved?

[RMeli]

Hi @ammarfitwalla, welcome to MDanalysis! We do not assign issues. You are very welcome to work on open issues (including this one) and open a pull request (PR) with your fix. Just make sure there is no other open PR against the same issue.

[paraglondhe098]

Is this issue closed ?

[lilyminium]

Hi @paraglondhe098, no, this issue is still open. However, it is understandably confusing as the two issues mentioned in the top comment have both been resolved. Currently NoDataError seems to be used in three major scenarios:

• a TopologyAttr is not present (e.g. bonds, charges)
• data is missing in an analysis class because run() has not been called (e.g. polymer, psa, dielectric, hbond_analysis)
• timestep data is missing (e.g. positions, velocities, forces)

It's also raised in the nojump transformation if there is no box in the universe.

Simply documenting this in the NoDataError class docstring would be a good start to resolving this issue. We should also document the case where it's not used as mentioned above, i.e. for zero-width arrays or empty AtomGroups.

[HeetVekariya]

• I understand this issue as to document, where NoDataError is used and where not(i.e. zero-width arrays or empty AtomGroups) in it's docstring.

• Should i use the text provided in the last comment for docstring (where it is used) ?
• what text should i use for NoDataError where it is not used (i.e. zero-width arrays or empty AtomGroups) ?

@lilyminium can you please help me with my queries ?

[lilyminium]

Hi @HeetVekariya, in both cases, yes, please clarify the docstring where you've screenshot. The issue here is that the current text is too non-specific -- having the docstring include where the error is raised or not raised (as I mentioned in my comment here #3901 (comment)) would be a major improvement.

[HeetVekariya]

@lilyminium

While doing the contribution and following Docs, i found that this should be improved,

1. I noted that there is typo in here, it should be 08/28/2023 according to the rules.

2. We should update this to conda create --name mdanalysis-dev "python=3.10", as current MDA version supports python 3.10 only. As i run the command given in the docs, it leads to error when dependencies were downloaded because conda environment is create with 3.12"
   

3. Formatting error is created here when authors.py is created from AUTHORS
   

• If these changes are needed should i create a separate PR for this ?
• If you are comfortable, can you share your discord handle so we can connect there too.

[RMeli]

1. It is not a typo, we use the DD/MM/YYYY format
2. MDAnalysis does not support Python 3.10 only. The develop branch supports 3.9, 3.10, and 3.11 (but not 3.12). Since 3.12 is not supported, I agree the installation command should be changed accordingly. For future proofing it might be worth changing it to conda crate --name mdanalysis-dev "python=3.Y" and point the user to the supported values of Y.
3. authors.py is a generated file, so this doesn't really matter. The solution would be to change the generation of the file to output a file without issues, but I don't think it is necessary

On a separate note, please avoid posting screenshots if not really necessary. Images are not searchable so it is much more useful to copy-paste what you want to show into a fenced side block.

[HeetVekariya]

1. It is not a typo, we use the DD/MM/YYYY format
2. MDAnalysis does not support Python 3.10 only. The develop branch supports 3.9, 3.10, and 3.11 (but not 3.12). Since 3.12 is not supported, I agree the installation command should be changed accordingly. For future proofing it might be worth changing it to conda crate --name mdanalysis-dev "python=3.Y" and point the user to the supported values of Y.
3. authors.py is a generated file, so this doesn't really matter. The solution would be to change the generation of the file to output a file without issues, but I don't think it is necessary

On a separate note, please avoid posting screenshots if not really necessary. Images are not searchable so it is much more useful to copy-paste what you want to show into a fenced side block.

• Will keep the note of the suggestions.
• The rule here says that format is MM/DD/YYYY.
• All the other dates are written in the MM/DD/YYYY format. If rule is changed then we might need to update it.

[IAlibay]

@HeetVekariya you are right this is a typo I made last release. However for your PR I would suggest just ignoring it.

There are some other historical changes I need to do (at least one instance of duplicate authors in the changelog), so I'll open a separate PR to deal with it.

[RMeli]

@HeetVekariya you are right, apologies. I was on my phone an did not double check, sorry for the confusion.

[HeetVekariya]

I understand, We are working collaboratively 👍