Skip to content

clean up the string output in read_nexus#712

Merged
lukaspie merged 8 commits intouse-log-files-in-testsfrom
str-output-in-read-nexus
Oct 13, 2025
Merged

clean up the string output in read_nexus#712
lukaspie merged 8 commits intouse-log-files-in-testsfrom
str-output-in-read-nexus

Conversation

@lukaspie
Copy link
Copy Markdown
Collaborator

@lukaspie lukaspie commented Oct 9, 2025

addresses question asked in #375 (comment)

@lukaspie lukaspie changed the title clean up the string output in read_nexus clean up the string output in read_nexus Oct 9, 2025
@lukaspie lukaspie marked this pull request as draft October 10, 2025 14:16
RubelMozumder
RubelMozumder reviewed Oct 13, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@RubelMozumder RubelMozumder left a comment

Choose a reason for hiding this comment

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

LGTM. I think the PR should be merged to the branch use-log-files-in-tests, then we can test both changes together.

RubelMozumder
RubelMozumder reviewed Oct 13, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@RubelMozumder RubelMozumder left a comment

Choose a reason for hiding this comment

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

Could we also add some tests, so in future if something is broken we can extnd the code and tests as well.

@lukaspie
Copy link
Copy Markdown
Collaborator Author

lukaspie commented Oct 13, 2025

LGTM. I think the PR should be merged to the branch use-log-files-in-tests, then we can test both changes together.

Sounds good, I have changed the target branch of this PR accordingly.

Could we also add some tests, so in future if something is broken we can extnd the code and tests as well.

Yeah, I'll work on those. I'll let you know when it's ready.

@lukaspie lukaspie changed the base branch from master to use-log-files-in-tests October 13, 2025 11:54
@lukaspie lukaspie marked this pull request as ready for review October 13, 2025 11:54
@lukaspie lukaspie force-pushed the str-output-in-read-nexus branch from 5c258c0 to 83bee88 Compare October 13, 2025 12:43
@lukaspie lukaspie changed the base branch from use-log-files-in-tests to master October 13, 2025 12:45
@lukaspie lukaspie changed the base branch from master to use-log-files-in-tests October 13, 2025 12:46
@lukaspie
Copy link
Copy Markdown
Collaborator Author

lukaspie commented Oct 13, 2025

@RubelMozumder I think we can merge this to the other PR. I added a bunch of test cases as well.

You don't see it here in the GitHub UI, but tests are passing: https://github.com/FAIRmat-NFDI/pynxtools/actions/runs/18464912935?pr=712 & https://github.com/FAIRmat-NFDI/pynxtools/actions/runs/18464912911?pr=712

RubelMozumder
RubelMozumder approved these changes Oct 13, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@RubelMozumder RubelMozumder left a comment

Choose a reason for hiding this comment

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

LGTM!
I have changed one missing quotation and changed one test value.

Comment thread src/pynxtools/nexus/nexus.py
Comment thread tests/nexus/test_nexus.py
@lukaspie lukaspie merged commit 57e47c2 into use-log-files-in-tests Oct 13, 2025
1 check passed
@lukaspie lukaspie deleted the str-output-in-read-nexus branch October 13, 2025 13:53
lukaspie added a commit that referenced this pull request Oct 13, 2025
@lukaspie
clean up the string output in read_nexus (#712)
12f9897
lukaspie added a commit that referenced this pull request Oct 20, 2025
@lukaspie
clean up the string output in read_nexus (#712)
a17b173
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RubelMozumder RubelMozumder RubelMozumder approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lukaspie @RubelMozumder