Port ctypesgen to argparse

[mara004]

This would address #159

It's still in an early state and I haven't tested most options yet, but the basic usage appears to work (and the test suite passes).

[mara004]

Thanks for re-running CI. Apparently black complains that I put multiple option names on one line.
Is there any way of making it tolerate that, or should I just apply the suggested patch?

[nilason]

Many thanks also for this contribution! I do not feel that alone anymore :-) .
I'd appreciate if you'd make Black content (even if I understand why you prefer keeping your one-line solution in this case).

[mara004]

I have now applied black's patch.

[Alan-R]

I've been listening to this conversation for a bit, and have some concerns about breaking existing code with radical (even if needed) changes. If this truly has no behavioral effects, then wonderful!! If not, then I would counsel caution concerning changes in something as basic and ubiquitous as strings. Don't misunderstand me - I dislike the current implementation... Is this something which can be turned off or on by the user? If not, can it be made so? Thanks!

…

On Sun, Nov 27, 2022, at 9:36 AM, Nicklas Larsson wrote: Many thanks also for this contribution! I do not feel that alone anymore :-) . I'd appreciate if you'd make Black content (even if I understand why you prefer keeping your one-line solution in this case). — Reply to this email directly, view it on GitHub <#161 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJTJI4ZAGJDEI7UP2QENLLWKOEYPANCNFSM6AAAAAASLUPSOA>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

-- Alan Robertson ***@***.***

[mara004]

@Alan-R I think you are referring to the other PR #162, right? Assuming that's the case:

If this truly has no behavioral effects, then wonderful!!

The strings PR does have intentional behavioral effects in that input strings need to be encoded excplicitly, and output strings need to be casted and decoded. It deliberately is a breaking change, and has been clearly described as such.

If not, then I would counsel caution concerning changes in something as basic and ubiquitous as strings. Don't misunderstand me - I dislike the current implementation...

Is this something which can be turned off or on by the user?

If not, can it be made so?

It's not optional ATM. I belive making it so would substantially complicate both the implementation and maintenance of ctypesgen.
Naturally, that is a change which needs to be well-considered, but if it's technically correct, I think it may finally be time to move forward. (There seems to be quite some technical debt in ctypesgen.)
For what the project I maintain is concerned (pypdfium2), I am not willing to carry around ctypesgen's displeasing string classes any longer, but would like to avoid working with a fork on the long run.

---

This PR here, on the other hand, should be much less controversial. It only moves away from a deprecated module and removes some code that is defunct (and perhaps always has been).

---

That said, I would be glad if other users of ctypesgen could review and test the two changes and give their feedback.
Of course, it is possible that I might have overlooked, misunderstood, or else accidentally broken something.

[TheCodeArtist]

That said, I would be glad if other users of ctypesgen could review and test the two changes and give their feedback.
Of course, it is possible that I might have overlooked, misunderstood, or else accidentally broken something.

Unfortunately, from what i have seen on this project,
there hasn't been much of user feedback/review in recent times.
Very few contributors/reviewers. 🙁
(though from the assorted issues, it is evident there are likely 100s of users of ctypesgen out there)

For what the project I maintain is concerned (pypdfium2),
I am not willing to carry around ctypesgen's displeasing string classes any longer,
but would like to avoid working with a fork on the long run.

Absolutely makes sense! ❤️
Thank you for clubbing together and being more efficient here
(instead of alternative forks or "hacky wrappers").

As a maintainer yourself,
i think you would understand the concern against any one-off "drive-by" changes,
especially ones that change behavior.

Here's an idea.
What if one (or more) of the recently active contributors...

• you (@mara004)
• @nilason
• @fgrie
• and me (@TheCodeArtist)

...can understand the parts being modified in this PR / PR #162 , to such an extent that,
after this PR / PR #162 is merged, if any issue is identified/reported for the next 6months ,
anyone among the 4 of us can quickly fix the issue.

...and also commit to fixing any issue in the near future that might pop-up due to this PR / PR #162.
If we do this, i think it might be easy for the maintainer to accept/merge this PR / PR #162.

@mara004 What do you think?
(as the author this PR / PR #162,
IMHO, you get first dibs 🙂 to say if you can commit to
some short-term maintenance of any issues that might arise from the PR)

---

PS: i will review the changes in this PR and PR #162 to the best of my knowledge this weekend.
and will add comment to gain more confidence in what continues to work as expected and what might break.
(i don't claim to be an expert, just one more data-point.)

[mara004]

@TheCodeArtist I understand the issue with "drive-by" changes and the quest for continued maintenance. But there are some problems with what you're asking for, too:

• It totally depends on the nature of the issue if it can be fixed, in which time frame, and by which developer.
• No one knows what will happen, so I can't make any promises.
• Free software also means that developers are free to decide what they want and don't want to do.

It is however my intent is to continue to help with ctypesgen while I need it and have the required time/energy/knowledge.

If any really serious problems should arise, then it's still possible to just revert the change and ask for a new PR to re-do it cleanly.

Another note: If the requirements for a contribution are set too high, ctypesgen will become even more outdated over time and might eventually become unusable (e. g. if Python decide to get rid of optparse).

[nilason]

I think this change to argparse is in perfectly in order. It is only a question of time before optparse will be gone. (A situation I found myself in recently with old code and Python 3.11). Besides there will not be no significant breaking changes for users.
@mara004 I will test this as soon as I can.

[TheCodeArtist]

It is however my intent is to continue to help with ctypesgen while I need it and have the required time/energy/knowledge.

Thank You. ❤️ That's all one can ask/hope for.
Agreed. No one can predict the future.

If any really serious problems should arise,
then it's still possible to just revert the change and ask for a new PR to re-do it cleanly.

Ya. Hopefully no other changes on top. 🤞
(likely not a challenge for ctypengen with its limited contributions)

ctypesgen will become even more outdated over time and might eventually become unusable

Absolutely!
Probably it almost went this way, before @nilason (and other major contributors) resurrected/modernized it 😅
Thanks for taking up the attempt to port from optparse to argparse. 👏

[mara004]

@nilason Thanks, take your time :)

[mara004]

FYI, I recently had a conversation with @pukkandan of yt-dlp,¹ who says that optparse will remain available for at least the next 5 years, probably more. So the situation is much less pressing than I initially thought.

Despite the warning on the documentation, PEP 594 lists optparse among the modules to keep ("Withdrawn deprecations"), and importing optparse does not even raise a deprecation warning. Apparently, it was/is disputed upstream whether optparse should be deprecated at all.

Footnotes

1. which incidentally still uses optparse as well and would be a lot of work to port ↩

[mara004]

This PR is open since Nov 2022, i. e. >6 months as of this writing. Any progress regarding the merge decision, also concerning #162?

The thing is, I'd like to make some more changes on behalf of pypdfium2 (e.g. pypdfium2-team/pypdfium2#215) that I'm not able to isolate into separate master-based branches as they interfere with the same code areas as these patches.

That said, the attempt to upstream the changes is getting an increasing burden, so I might eventually prefer to work independently in a fork that would enable me to apply more radical changes at a higher pace.

[nilason]

This PR is open since Nov 2022, i. e. >6 months as of this writing. Any progress regarding the merge decision, also concerning #162?

I'm awfully sorry for this totally unintended delay into looking looking at this. Please rebase after merge of #166 .

The thing is, I'd like to make some more changes on behalf of pypdfium2 (e.g. pypdfium2-team/pypdfium2#215) that I'm not able to isolate into separate master-based branches as they interfere with the same code areas as these patches.

That said, the attempt to upstream the changes is getting an increasing burden, so I might eventually prefer to work independently in a fork that would enable me to apply more radical changes at a higher pace.

That would be very unfortunate, better to focus the VERY few dev resources available/dedicated in one place. If nothing unexpected falls from above, I'll look at this the next few days.

[mara004]

Thanks, didn't expect such a quick reply!

Unfortunately I now realize I have already gone down experimenting in my pypdfium2 branch a bit too deeply.
Please give me time to sort out the situation, I'm somewhat overwhelmed TBH.
Ideally, could you do the rebasing so we can at least get over with this change?

I can yet try to split out upstreamable patches for pypdfium2-team/pypdfium2#178
and pypdfium2-team/pypdfium2#215 eventually, but otherwise I fear this will be very difficult to handle.

The problem is that ctypesgen has become a bit of a mess, so theoretically I'd like some cleanup and more radical, backwards incompatible changes, but that would be controversial and extremely hard to do with distinct PRs, meaning a lot more work and delay.

[nilason]

Ideally, could you do the rebasing so we can at least get over with this change?

It was a pretty straight forward rebase, with the minor addition of following changes:

--- a/ctypesgen/__main__.py
+++ b/ctypesgen/__main__.py
@@ -243,8 +243,7 @@ def main(givenargs=None):
         default=False,
         help="Do not support extra C types built in to Python",
     )
-    op.add_option(
-        "",
+    parser.add_argument(
         "--no-load-library",
         action="store_true",
         dest="no_load_library",
-- 

but for some, to me, unknown reason I wasn't able to push the commits to your fork.

Otherwise it looks good to me and passed all test I could throw at it without regression.

[mara004]

but for some, to me, unknown reason I wasn't able to push the commits to your fork.

That's strange, I don't know the reason either. I almost always permit pushes by project members when submitting a PR.
In that case, I'll do the rebase and push shortly.

[mara004]

Done, hope it works now. When you merge, please squash the commits. Thanks!

[mara004]

@nilason

[nilason]

@mara004 Thanks for your contribution!

[mara004]

Thanks for merging!

[mara004]

Turns out python list += actually mutates the list on the left (instead of making a new list), which can be problematic when ctypesgen is called repeatedly through the API: If the default [] is used, it is modified, so the next run's default compile/runtime libdirs would retain the previous run's universal libdirs.

Python's behavior of a += b modifying while a = a + b does not feels slightly confusing, though. Normally you'd expect both syntaxes to do the same thing.