Skip to content

Add filename property #1571

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

Merged
merged 20 commits into from
Jul 2, 2025
Merged

Conversation

PeterNSteinmetz
Copy link
Contributor

Adds the OriginalFileName property to the NlxHeader. Also cleans up private members and moves a useful method to NlxHeader from test.

This is the last in this sequence of commits for a while now. This now provides a much clearer method of parsing the header which works with all extant files (@JuliaSprenger and thus provides that refactoring discussed a while back ;-). It should be more flexible for future possible variations and easier to modify if needed.

If one wanted to squish all of these since the divergence to master there would be much cleaner code to review, however, it would be major changes in one shot.

@zm711
Copy link
Contributor

zm711 commented Sep 27, 2024

Thanks Peter. Sam said he would review this when he has some time. So I assigned him to all of these and he will be the main reviewer.

@samuelgarcia
Copy link
Contributor

Hi Peter,
I read the long series of PR that depend of the previous ones.
I am OK with all changes.

Because of conflict I propose to merge only this last one that should include all of then.
In short merging this one should include also:
#1562
#1563
#1566
#1567
#1568

I will push the fix myself.

@samuelgarcia
Copy link
Contributor

@zm711 : this is ready to merge on my side.

Copy link
Contributor

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

Works for me. Really appreciate the comment with all the different file header types to keep track of everything.

@zm711 zm711 merged commit c664dd5 into NeuralEnsemble:master Jul 2, 2025
5 checks passed
@zm711 zm711 mentioned this pull request Jul 2, 2025
@zm711 zm711 added this to the 0.14.2 milestone Jul 2, 2025
zm711 added a commit to zm711/python-neo that referenced this pull request Jul 2, 2025
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.

3 participants