find_data in lark

[amorshedi]

self.ast.find_data('atom') destroys the original order of atoms in a SMARTSGraph. Since later on, the first atom is used for atom-typing, it was causing an error. The following keeps the order:
[x for x in self.ast.iter_subtrees_topdown() if x.data == 'atom']

PR Summary:

PR Checklist

---

• Includes appropriate unit test(s)
• Appropriate docstring(s) are added/updated
• Code is (approximately) PEP8 compliant
• Issue(s) raised/addressed?

[codecov]

Codecov Report

Merging #341 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #341   +/-   ##
=======================================
  Coverage   84.55%   84.55%           
=======================================
  Files          14       14           
  Lines        1282     1282           
=======================================
  Hits         1084     1084           
  Misses        198      198           

[daico007]

Nice fix! Can you also unpin the lark-parser version in requirements.txt and requirements-dev.txt so we can test if this will fix the issue foyer has with work with lark-parser 0.8.6 ?

[amorshedi]

@daico007 I'm a bit new to github. Is this what you mean:
I should go to my fork of foyer and change lark-parser <=0.8.5 to lark-parser in both requirements.txt and requirements-dev.txt and then commit?

[daico007]

@violet101 Yes, that's what I meant.

[amorshedi]

@daico007 Done. Thanks.

[rmatsum836]

Thanks @violet101 for your contribution! I was stumped on this issue and I really appreciate you figuring it out!

[daico007]

LGTM! Thank you for your contribution!

[justinGilmer]

Thank you for this helpful fix @violet101 !

I noticed that the pull request was based off of your master branch, traditionally that might cause issues in the future when, for example, you wanted to work on two different fixes for foyer at the same time. Normally you would want to keep each of those changes separate so the features and pull requests are not too large and difficult to understand (that does not really apply here, but in the future it might).

The way we prevent those issues for our codebases here is through the fork and pull request model

You have already done part of that, by forking foyer, which is awesome!

The other crucial piece for our pull requests are using feature branches to submit your changes

We use the same remote names as defined in the first link:

• upstream is the repository you forked off of, in this case: mosdef-hub/foyer.
• origin is the name of your forked repository, here: violet101/foyer

If you have any issues setting this up, please reach out to any of us, we can be found at our Gitter channel: https://gitter.im/mosdef-hub/Lobby

[amorshedi]

@rmatsum836 @daico007 I'm glad it helped :)

@justinGilmer Thanks for the helpful tips.