Skip to content

Update requirements to be more specific#752

Merged
aaronayres35 merged 2 commits into
maint/5.0from
update-requirements
Jun 3, 2021
Merged

Update requirements to be more specific#752
aaronayres35 merged 2 commits into
maint/5.0from
update-requirements

Conversation

@aaronayres35
Copy link
Copy Markdown
Contributor

@aaronayres35 aaronayres35 commented Jun 3, 2021

[targeting maint/5.0]
This PR updates __requires__ to be more specific.
We use Property(observe=...) and so depend on traits >= 6.2. Also recent changes are such that we require enable 5.2 (only rc currently available). Also enable 5.2 (and actually I think 5.1) require pyface >=7.2 as it uses pyface.undo. So, chaco implicitly requires pyface >= 7.2
TBH I am not exactly sure on what traitsui is necessary (we didnt specify for enable either)

This PR will eventually need to be backported into master along with the recent changelog updates

Comment thread chaco/__init__.py Outdated

__requires__ = ["traits", "traitsui", "pyface", "numpy", "enable"]
__requires__ = [
"traits>=6.2.0", "traitsui", "pyface>=7.2.0", "numpy", "enable>=5.2.0"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I should check that a release candidate actually counts for >= . Otherwise even with the enable 5.2.0rc2 out, there will not be available packages on PyPI satisfying the requirements when we make the rc for chaco.

If that is the case, I will unpin this requirement until after the chaco rc is made, and after enable 5.2.0 is fully released, but before chaco 5.0.0 is release

@aaronayres35 aaronayres35 requested a review from rahulporuri June 3, 2021 11:57
@aaronayres35 aaronayres35 mentioned this pull request Jun 3, 2021
44 tasks
@rahulporuri
Copy link
Copy Markdown
Contributor

Also recent changes are such that we require enable 5.2 (only rc currently available).

I'm not a 100% sure why enable 5.2 is required. This will only be true after #674 IIUC. Right?

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

Almost LGTM.

I haven't tested this locally but what happens if you just pip install chaco from source in the maintenance branch now? A 5.2.0 doesn't exist in pypi yet and i'm wondering if the installation fails.

@rahulporuri
Copy link
Copy Markdown
Contributor

I haven't tested this locally but what happens if you just pip install chaco from source in the maintenance branch now? A 5.2.0 doesn't exist in pypi yet and i'm wondering if the installation fails.

This doesn't work as I expected. I see the following error when I try to install a chaco sdist in a clean environment -

(test-chaco) >python -m pip install chaco-4.8.1.dev200.tar.gz
Processing c:\users\rporuri\work\github\ets\chaco\dist\chaco-4.8.1.dev200.tar.gz
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
ERROR: Could not find a version that satisfies the requirement enable>=5.2.0 (from chaco) (from versions: 4.4.0, 4.4.1, 4.5.0, 4.5.1, 4.6.0, 4.6.1, 4.6.2, 4.7.0, 4.7.1, 4.7.2, 4.8.0, 4.8.1, 5.0.0rc2, 5.0.1, 5.1.0rc1, 5.1.0rc2, 5.1.0, 5.1.1, 5.2.0rc2)
ERROR: No matching distribution found for enable>=5.2.0

@aaronayres35
Copy link
Copy Markdown
Contributor Author

I haven't tested this locally but what happens if you just pip install chaco from source in the maintenance branch now? A 5.2.0 doesn't exist in pypi yet and i'm wondering if the installation fails.

This doesn't work as I expected. I see the following error when I try to install a chaco sdist in a clean environment -

(test-chaco) >python -m pip install chaco-4.8.1.dev200.tar.gz
Processing c:\users\rporuri\work\github\ets\chaco\dist\chaco-4.8.1.dev200.tar.gz
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
ERROR: Could not find a version that satisfies the requirement enable>=5.2.0 (from chaco) (from versions: 4.4.0, 4.4.1, 4.5.0, 4.5.1, 4.6.0, 4.6.1, 4.6.2, 4.7.0, 4.7.1, 4.7.2, 4.8.0, 4.8.1, 5.0.0rc2, 5.0.1, 5.1.0rc1, 5.1.0rc2, 5.1.0, 5.1.1, 5.2.0rc2)
ERROR: No matching distribution found for enable>=5.2.0

Yeah I just caught that myself, this is what I was unsure about in the PR description. I will unpin this now, but we may want to repin it later (post chaco rc and enable full release)

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM. We'll need to remember to change this once enable 5.2.0 is released 😅

@aaronayres35
Copy link
Copy Markdown
Contributor Author

I'm not a 100% sure why enable 5.2 is required. This will only be true after #674 IIUC. Right?

Looking closer I believe that is correct, I don't know why I had it in my head there was something else. I was jumping the gun too early with this PR

@aaronayres35 aaronayres35 merged commit edf0faa into maint/5.0 Jun 3, 2021
@aaronayres35 aaronayres35 deleted the update-requirements branch June 3, 2021 12:25
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