Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented May 11, 2018

In many cases, the registered PyPI name will be the same as package/module name, and we will usually be able to construct that from the passed path. This commit fills in the path-based value as a default, so users who do set __title__ explicitly will be able to clobber that default with their custom value.

In many cases, the registered PyPI name will be the same as
package/module name, and we will usually be able to construct that
from the passed path.  This commit fills in the path-based value as a
default, so users who do set __title__ explicitly will be able to
clobber that default with their custom value.
@reubano
Copy link
Owner

reubano commented Jun 10, 2018

I like this patch, but to prove it doesn't break anything, you mind editing the test to remove __title__ = 'pkutils'?

Personally, I prefer the explicit __title__ in this case, because:

* It shows how you'd set __title__ if you wanted to, and
* The random name from the temporary file is probably not a great
  title.

But Reuben wants this for testing the new code [1], so here it is ;).

[1]: reubano#2 (comment)
@wking
Copy link
Contributor Author

wking commented Jun 18, 2018

I like this patch, but to prove it doesn't break anything, you mind editing the test to remove __title__ = 'pkutils'?

I've pushed 5e88b7c with that change, but it's a bit awkward for the randomly-named test module. I'm fine with or without the doctest commit.

@reubano reubano merged commit d865981 into reubano:master Jun 27, 2018
... module = parse_module(f.name)
... module.__version__ == '0.12.4'
... module.__title__ == module.get('__title__') == 'pkutils'
... module.__title__ == os.path.splitext(os.path.basename(f.name))[0]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like I made this line too long for lint. @reubano, it's probably easier for you to fix that directly, but I can file a fixup PR if it would help.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm workin on it. No worries.

@wking wking deleted the optional-__title__ branch June 27, 2018 20:29
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.

2 participants