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

Additional parsers #1302

Merged
merged 6 commits into from
Dec 4, 2023
Merged

Additional parsers #1302

merged 6 commits into from
Dec 4, 2023

Conversation

amandadumi
Copy link
Contributor

This incorporates the skip_line functionality from logfileparser as a class function into file_handler. This function might only ever be used in a virtual way, so the extra argument I've added might not be needed, but I wanted to be safe.

This is adding atomnos and atomcoords.

@amandadumi
Copy link
Contributor Author

Questions:

  1. Should i be formatting with black now?
  2. Can we discuss what is the plan for porting tests? (or if this was discussed and I'm forgetting, please remind me.)

@shivupa
Copy link
Contributor

shivupa commented Nov 28, 2023

Questions:

1. Should i be formatting with black now?

Let's do this once on main at the end.

2. Can we discuss what is the plan for porting tests? (or if this was discussed and I'm forgetting, please remind me.)

This needs to be a topic of discussion on tuesday

cclib/combinator/combinator.py Show resolved Hide resolved
cclib/parser_properties/atomcoords.py Show resolved Hide resolved
@berquist berquist self-requested a review December 4, 2023 16:51
@@ -351,3 +351,56 @@ def finish(self):

self.file_pointer = len(self.files) -1
self.pos = self.size

def skip_lines(self, sequence: typing.Iterable[str],virtual=False) -> typing.List[str]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this is just a port.

@berquist
Copy link
Member

berquist commented Dec 4, 2023

Formatting will be done after the rebase.

Tests I'm still not sure how this will work with the new pytest stuff which I increasingly think should be merged into master and not main.

@berquist berquist merged commit 0909911 into cclib:main Dec 4, 2023
1 of 5 checks passed
@berquist berquist added this to the v2.x milestone Dec 15, 2023
@shivupa shivupa mentioned this pull request Jan 8, 2024
@berquist berquist modified the milestones: v2.0, v2.0a Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants