Add #[name = "foo"] attribute to #[pymethods]

[davidhewitt]

Closes #663

To achieve this, I added to FnSpec:

• a new field name - the name of the Rust function this maps to
• a new field python_name - either the optionally-set #[name], or the .unraw() rust name

While I was at it I did some refactoring to swap some panic!-s to spanned errors, and a bit of cleanup.

TODO:

• Bikeshed name - #[pyname] perhaps?
• Support #[pyfunction]?
• Update documentation

[davidhewitt]

Bikeshed: maybe the attribute should be #[pyname] instead of #[name]?

[programmerjake]

#[pyname] could work, on the other hand, we already have #[text_signature] and not #[pytext_signature], however #[name] is much more likely to conflict with something in the future due to being a much more common word than #[text_signature]

[programmerjake]

Other than the issues mentioned, it looks good!

[programmerjake]

Looks good

[kngwyu]

Thank you!
Though I have a concern about this feature itself...(isn't it confusing to give different names to Python API?), this PR is generally well written and refactorings are good 👍
But, if possible, I want you to avoid refactoring codes unrelated to the PR.
It makes code review more difficult and introduces unnecessary merge conflicts.

[davidhewitt]

Though I have a concern about this feature itself...(isn't it confusing to give different names to Python API?)

I agree most users won't need this. There will be edge cases like the example originally supplied on #663.

Note that the #[pyfn] macro actually requires a python name as the second argument. So adding this #[name] attribute brings the other macros to feature parity without forcing all users to specify names.

It's probably too breaking but I now actually think it would be nice to remove the required name argument from #[pyfn] and replace it with this optional attribute.

[davidhewitt]

I've pushed changes to all the things you suggested.

But, if possible, I want you to avoid refactoring codes unrelated to the PR.
It makes code review more difficult and introduces unnecessary merge conflicts.

Sorry. I'll do refactorings as separate PRs next time 😄

[programmerjake]

For #[getter] and #[setter], maybe it would be worthwhile to have the error message be something like: "#[name] can't be used with `#[getter]`, to override the Python name, use `#[getter(python_name)]`"

alternatively, for consistency, just accept:

#[getter]
#[name = "python_name"]
fn rust_name(&self) -> i32 {
    0
}
#[getter(python_name2)]
fn rust_name2(&self) -> i32 {
    0
}

but not:

#[getter(other_python_name)]
#[name = "python_name"]
fn rust_name(&self) -> i32 {
    0
}

[davidhewitt]

For #[getter] and #[setter], maybe it would be worthwhile to have the error message be something like: "#[name] can't be used with #[getter], to override the Python name, use #[getter(python_name)]"

I actually did have different error messages (not quite so detailed as that), but removed it on kngwyu's recommendation 😄

alternatively, for consistency, just accept: [...] but not: [...]

I did consider this too, but I felt that it's better to have just one way to set the python name for a getter / setter. If the consensus is to enable #[name] for getters & setters, and only fail when it's set both ways, then I can change this PR to do that.

[kngwyu]

Thank you!

alternatively, for consistency, just accept:

I often feel this kind of design decision difficult.
Let's decide if it's good or not after using the current interface for a while.
If some users complain about this, then we should change the interface.