Update python interface with rev2 features

[achaikou]

Python 3.9 removed in this PR because original run failed for some reason, no clue why.

benchmarks are failing on my test runs.
header retrieval is slower for some reason, but most importantly "create" is way slower.
Looks to be caused by "Update Field to support trace headers" commit.

I don't see anything stupid, but please let me know if you do. We need to dig deeper as it is probably not a random failure.

Upd:
I managed for now to get slowdown to a more reasonable percentage, but the problem still stays.
Apparently __setattr__ (and maybe __getattr__ , didn't check that or the other classes with .name support) is just slow.
Simply defining it adds 25% slowness to 'create' benchmark test.

We need to discuss and find out if this is a deal breaker for our interface as Field is a main class responsible for fields extraction.

[ajaust]

This looks very nice overall. 🥳 I left some suggestions or questions here and there. Mostly about the docstrings. I also build the Sphinx documentation locally. The generated documenation looks like expected.

Regarding the failing test in Python 3.9

It complains about FAILED test/segy.py::test_field_via_traceheader - Failed: DID NOT RAISE <clas... I am not sure if we should be worried that some expected exception is not thrown, but I am also not sure if investigating it further makes sense.

Building with Sphinx

I build it with Python 3.14 and ran into the following issues

• requirements-doc.txt does not specify the ReadTheDocs theme as dependency. We would need to add sphinx-rtd-theme in the dependency file or one needs to use requirements-readthedocs.txt where the dependency is specified. We could also consider consolidating both the requirement files into one.
• Building the docs on failed since conf.py uses distutils for LooseVersion. distutils was deprecated in Python 3.10 and removed in 3.12. I replaced it with packaging.version.Version which resolved the issue.

[ajaust]

What is the difference between an int and a number? Could it be any number type like float?

[achaikou]

Yes, that was the idea.
Hm. I was absolute sure this is a standard notation, so I didn't even check.
No idea where I took it from.

I think I should explicitly change to it int, float,

[ajaust]

I also think I have see number and number-like so from my side you are free to decide what the best type is to describe the return type.

[achaikou]

I changed it to ints and floats.

[achaikou]

Thanks for the blazing fast review! 🔥

Python 3.9: I don't see a point in investigating it unless someone thinks it is useful.

Sphinx:

• last commit that touched requirements-doc.txt introduced it on purpose. I am not too deep into it to say if it was warranted or not, so I am inclined to leave it as is.
• thanks for the investigation! Applied.

---

Performance

So running benchmark create test with f.traceheader[tr][0].update({d}) is roughly 50% slower than f.header[tr].update({d}).

Reading this

with segyio.open(filepath) as f:
    for tr in f.ilines * f.xlines:
        f.traceheader[tr][0][189]

vs

with segyio.open(filepath) as f:
    for tr in f.ilines * f.xlines:
        f.header[tr][189]

is around 2 times slower.

From what I see slowness happens simply from default definition of __setattr__ (probably __getattr__ too) for TraceHeaderSequence and TraceHeaderField interfaces, though I can't find any similar complaints online. I am running with 3.13 and all the issues seem to relate to 3.11
Maybe those are just fluctuations? Doubt that, given that it affected header test that is index-based, but still...

I am still not convinced we make the right decision to leave it be, because if slowness is real the only reasonable way to improve the performance is to remove the functions altogether, which would be a major breaking change for the users.

It probably must hold though, given that we don't have much time, but we need to highlight that in pre-release notes.

[achaikou]

Yes, that was the idea.
Hm. I was absolute sure this is a standard notation, so I didn't even check.
No idea where I took it from.

I think I should explicitly change to it int, float,

[ajaust]

Great! Thanks for the fixes. It looks good to me. I just left one or two comments as "unresolved" such that my reply is easier to find. I consider everything as resolved. I also really like that you updated the Sphinx build process. In this sense: "Approved" 🎉

I understand that the performance issue is a potential problem. Do you have some pointer to the issues that were reported in Python 3.11? Should the performance issue also show up in our benchmark tests? At least the most recent benchmark run passed successfully.

[ajaust]

I also think I have see number and number-like so from my side you are free to decide what the best type is to describe the return type.

[ajaust]

I just realised that performance has probably improved due to the introduction of TraceHeaderField in a fixup.

[yngve793]

In general the PR is fine. I have no strong objections. Some minor comments and suggestions are added. The only point I want to stress is the naming of classes in the python/segyio/trace.py file. The current naming schemes works reasonably fine, but provides little conceptual understanding to the reader. What about something along these lines:

• TraceFieldEntry : Name, offset, type and value for a field
• TraceFieldSequence, TraceFieldList, TraceFieldDefinition, etc: Trace header definition as Standard, Extended etc.
• TraceFieldLayout : A sequence of TraceFieldDefinition containing Standard, Extended and private for a specific trace.
• TraceFieldOverview: Sequence of TraceFieldLayouts for all traces.

At least consider something along this lines.

[achaikou]

Thanks! 😻

Name changes and other stuff is done directly and not in fixups, as we agreed.
Hopefully I updated everything correctly 😄

[achaikou]

I changed it to ints and floats.

[ajaust]

Looks good to me. I only found 2 minor things. 😀

I did not check the documentation website generate by sphinx after the rebase/squash yet. Just give me a sign if you want me to do that. 🔍

[ajaust]

I just verified that the docstrings appear in the Sphinx documentation. I found some minor things (I am sorry) and added them to the review.

[achaikou]

Thanks for finding new typos! 🎉

[yngve793]

Fixups looks fine to me.
No further comments.

[ajaust]

Looks good. 🎉
Go ahead and apply the fixups and merge.