Parent.__call__

[videlec]

At https://github.com/fredstro/hilbertmodgroup/blob/main/src/hilbert_modgroup/upper_half_plane.pyx#L236 and https://github.com/fredstro/hilbertmodgroup/blob/main/src/hilbert_modgroup/hilbert_modular_group_class.py#L160 you overwrite the default __call__ of the sage Parent class. Is there any reason for that? Doing so would prevent any coercion to happen.

Part of openjournals/joss-reviews#3996

[fredstro]

I was actually intentionally trying to avoid Sage's coercion framework since I find it extremely mes for extension types (unless they happen to fit very well within an existing category). For instance, I couldn't make coercion work without adding a "construction" function which just returns None...

However, I took some time to do it and have now added explicit coercion to the ComplexPlaneProduct__class and UpperHalfPlaneProduct __class and their respective Element classes.
I am sure that someone that knows this better can improve on the logic but it works as intended now with all builtin arithmetic functions

[fredstro]

@videlec Is this OK?

[videlec]

You still have an implementation of __call__. Is that intentional?

[fredstro]

@videlec where do you see that?
The only call I see is a _call_ in the AnytoCPP class and I think that has to be implemented.

[fredstro]

Maybe you checked before I pushed the latest?

[videlec]

I was looking at the main branch. It is very confusing that the current branch is develop and not main. If you keep that setup write it down in the README.

[fredstro]

My current workflow is to use the develop branch for development and bugfixes and once they are verified as "correct" I will merge into main.

[fredstro]

Once / if there are more develpers involved I will use a more refined git-flow -based approach with feature branches.

[fredstro]

I know that releases re tagged but in my (perhaps wrong) opinion the "main" branch should always be as functionally correct as possible while the develop or feature branches can contain flaws to share between developers for comments and improvements.

[videlec]

This is non-sandard in github development (but not a problem). The more standard workflow is

• write code in a temporary branch and open merge request
• merge in main once the CI passed

You should describe your workflow in the README and be more specific on what users are expected to do with their merge requests (eg should we base our code on main or develop?)

[fredstro]

I see.
In my experience (as a consultant working on a few slightly larger but not massive commercial projects) the workflow that seems to be common in industry is to use short lived feature branches, based of a develop branch and once fnished they are merged back into the development branch. (This is the default git-flow configuration, only hotfixes are supposed to go directly into main)

The reason for this is that the development branch can contain many different new features that also have to be tested to work together and might not be developed in sequence.
Once the current develop branch (contaning a number of new features) is confirmed as working by QA (or a manager) it can be merged into main as part of the next release.

I will update the README.