Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow getting version despite deps missing #382

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

flying-sheep
Copy link
Contributor

@flying-sheep flying-sheep commented Dec 3, 2020

Also gets rid of the use of deprecated methods

@flying-sheep
Copy link
Contributor Author

@takluyver opinions?

@flying-sheep
Copy link
Contributor Author

@takluyver what do you think?

@takluyver
Copy link
Member

Hi @flying-sheep, sorry it's taken me ages to look at these PRs (this and #374). I know I'm rather neglecting my role as a maintainer of Flit - when I get a bit of time to do non-work programming, there's generally something more either more fun or where I feel I can make more concrete progress in a small chunk of time.

I like this idea better than #374. I don't love it - putting the __version__ definition above any significant imports is a bit of a 'one weird trick', but it makes getting the version work in more situations without the module having to do anything Flit specific, and I can see it can give better error messages when it fails, which is always good. I see it uses something new in Python 3.5, though - I'm still trying to support 3.4 in flit_core at present. Bumping the minimum Python version for flit_core means leaving any modules which want to support that version stuck on an older flit_core, which is a support headache, so I'm quite slow to do so, but even so, maybe it's time to let 3.4 go now.

I hope you won't mind if I take this chance to run an idea past you for making getting the version number more flexible. This is for discussion, at the moment, rather than implementation, but it's been bouncing around in my head for a while. I'll start with an illustration:

[tool.flit.version]
method = "import"
module = "foobar.version"

I.e. you would specify whether to use static analysis or execution, and which module defines the version. Questions I'm not sure about:

  • Import a module or exec a file? Exec-ing allows a foobar/version.py to be run without foobar/__init__.py, but importing is obviously how modules are normally loaded, and I place quite a high value on interacting with user code in normal, predictable ways.
    • I could make both methods available, but I've always tried to avoid adding too many options, so I don't want to do this.
  • Should it have an option to look for another name besides __version__?
  • Should static analysis be able to handle tuples like version_info = (3, 2, 0, 'beta')?
  • Should I go all in and allow specifying a function to call to retrieve the version number?

I imagine that you'll want maximum flexibility & power on all of these questions, whereas my instinct is to keep things simpler. I'm thinking through how many options I want to enable here. 🙂

I'm also thinking that if I do go for this, then the current behaviour of trying static analysis and then falling back to running code will eventually go away (after a suitable deprecation), and you will have to specify explicitly that you want it to import/exec code to get the version number.

@flying-sheep
Copy link
Contributor Author

flying-sheep commented Jul 7, 2021

Sounds great that you are thinking about tackling this!

I’m personally not maintaining things too far back: The slowest Linux distros are on something like 3.7 these days, right? … idk what reasons exist to support 3.4 for things that aren’t build tools. In flit’s case, “this is a build tool and other people still use 3.4” is an entirely valid reason if that’s how it is though!

I imagine that you'll want maximum flexibility & power on all of these questions, whereas my instinct is to keep things simpler.

I love simplicity! Power and simplicity aren’t mutually exclusive: Allowing people to call into whatever and clearly telling them that it’s not a flit bug if things go wrong is maximally powerful and very simple.

I personally want whatever solution allows my packages to robustly tell the truth about their version. And hardcoding a version isn’t the truth, unless you manually bump that version in every commit (which isn’t realistic). Of course other people have other philosophies here!

Questions I'm not sure about: […]

So what use cases do exist?

  1. some people hardcode __version__
  2. some people call code which depends on build-system.requires

The problem with use case 2 is that flit executes the root module before it installs all dependencies, which means that people with use case 2 can’t both use flit and import anything in their root module (unless they use my hack).

To truly support use case 2, we need to either

  1. give people a way to execute the version-generating code without executing the root module, or
  2. install all dependencies before executing the root module (Runtime requirements may be necessary at build time #141)

There’s three ways to run code without (fully) executing the root module:

  1. This PR: Try to execute the root module and pick __version__ out even when an ImportError happened
  2. Specify an out-of-module script/module to run/import (could be either a third party module or a standalone script living next to the module)
  3. Exec-ing a in-module script as if it wasn’t a submodule

So all in all I see 4 solutions: ensure all runtime dependencies are installed before executing the module or the three above.

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