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

Added 'TagString' test #1319

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added 'TagString' test #1319

wants to merge 2 commits into from

Conversation

patilatharva
Copy link
Collaborator

This test ensures tag objects have the string representation: Tag(time=$time, microstep=$microstep)

@patilatharva patilatharva requested a review from Soroosh129 August 4, 2022 18:02
@patilatharva patilatharva self-assigned this Aug 4, 2022
@Soroosh129
Copy link
Contributor

Soroosh129 commented Aug 9, 2022

Hi @patilatharva

It looks like the test you've added is failing with the following error:

expected tag string representation: Tag(time=$time, microstep=$microstep), instead got: {tag_str}

Do you know why that might be?

@patilatharva
Copy link
Collaborator Author

This seems to be because the python target submodule wasn't updated. The git history is a bit messy now but I just updated so the test should pass. Also, thanks to your comment I noticed a formatting error in the error message, which I fixed as well.

@Soroosh129
Copy link
Contributor

This seems to be because the python target submodule wasn't updated.

Ah I see.

The git history is a bit messy now.

No worries. We an squash the commits. Thanks for adding this feature!

@lhstrh
Copy link
Member

lhstrh commented Apr 14, 2023

@patilatharva: I looks like this PR has been under the radar for a while. Shall we bring it back into a state to where we can merge it? One issue with this PR is that I don't quite understand what it attempts to do, so if you could update the description to clarify, that would be a great first step. Thanks, and sorry for the delayed handling of this!

@patilatharva
Copy link
Collaborator Author

Yes, thanks for bringing this back to my attention! I'll make it ready for review.

@lhstrh lhstrh requested review from petervdonovan and removed request for Soroosh129 June 10, 2023 06:15
@lhstrh lhstrh removed the request for review from petervdonovan November 5, 2023 20:15
@lhstrh lhstrh added the testing label Nov 5, 2023
@lhstrh lhstrh requested a review from petervdonovan November 8, 2023 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants