Skip to content

GitHub Actions, Python 3.9+ Support, Jupyterlab Extensions #991

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 64 commits into from
Jun 9, 2023
Merged

GitHub Actions, Python 3.9+ Support, Jupyterlab Extensions #991

merged 64 commits into from
Jun 9, 2023

Conversation

srivarra
Copy link
Contributor

@srivarra srivarra commented May 11, 2023 •
edited
Loading

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Adds a Conda environment using Python 3.11.
The docker environment now uses a slim Python 3.11 Image.

How did you implement your changes

Environment Changes

Added the environment.yml file, adjusted the Dockerfile, made changes to the README and reformatted the setup.py file.
Support for Python 3.9 to 3.11.

Added the following extensions:

These extensions are not dependencies of Ark, they are installed alongside Ark in the Conda environment.yml and the Docker image Dockerfile, and can be optionally installed with pip install ark-analysis[lab_ext] or pip install .[lab_ext] if you are in the ark directory.

Testing Changes

  • Added a Python Script which runs as the first job in the CI workflow. This will grab the huggingface-dataset cache, and will verify and update it if necessary. (In the future we can add a drop down menu for us to be able to select a specific commit when working with PRs which require dataset changes). The cache is now cross-os enabled, so 1 cache for all tests. Tests are only able to read the cache, not write to it.
  • The fiber segmentation tests do not require the example dataset anymore, so these should be a bit faster.
  • Reduced the cell count for dimensionality_reduction_test.py from 300 to 50.

Removed unused imports in src/ark/, tests/, and templates/

Sorted all imports with isort, removed unused imports.

Remaining issues
Merge the following before this pr:

@srivarra srivarra linked an issue May 11, 2023 that may be closed by this pull request
Document supported python versions, expand versions #965
Closed
@srivarra srivarra self-assigned this May 11, 2023
@srivarra srivarra added the dependencies Pull requests that update a dependency file label May 11, 2023
Sricharan Reddy Varra added 22 commits May 12, 2023 11:21
testing windows path fix
0351037
set cell_cluster_col type to np.int64
c7e6c34
set cell_cluster_col type to np.int64 v2
d8577bb
dtype for pandas merge_asof explictly converted
92d60cb
dtype for pandas merge_asof convert to int
68414b2
convert to int
88a3081
huggingface cache
7dfe1f9
shutil fix
f21c0b6
change type to int
10d6495
changed a type to int
80f9d1c
example dataset cache download script
8a488d9
moved getting the dataset from test to ci workflow file
c9247ea
pycodestyle
8dbab1e
cache in ci fix
1501160
python-version fix
0b729df
correct data path
4c68ff5
caching logic updates
ffb2b80
save / restore cache
3d77455
set cache dir to ./data/cache
2c743ce
removed exampled dataset download in fiber_segmentation_test
489ef59
windows caching test
fdf4d5a
remove misc notebook metadata
0c49f73
ngreenwald
ngreenwald requested changes May 17, 2023
View reviewed changes
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

We should get the sequencing right for all of this. If @cliu72 wants the updated version for her paper to to be 3.9+, then we should wait on merging in #985 until after this.

If not, then we should make a new release once her other requests get merged in, then #985, then this.

@cliu72
Copy link
Contributor

cliu72 commented May 18, 2023 •
edited
Loading

We should get the sequencing right for all of this. If @cliu72 wants the updated version for her paper to to be 3.9+, then we should wait on merging in #985 until after this.

If not, then we should make a new release once her other requests get merged in, then #985, then this.

Yeah I think it'd be good for the updated version of my paper to be 3.9+, since pip install doesn't work anymore with the old version. So I think we can merge this one now.

camisowers
camisowers requested changes Jun 5, 2023
View reviewed changes
Copy link
Contributor

@camisowers camisowers left a comment

Choose a reason for hiding this comment

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

Minor comment about the alpineer version, but other than that it looks great.

@srivarra srivarra requested review from ngreenwald and camisowers June 7, 2023 21:19
@srivarra
Copy link
Contributor Author

srivarra commented Jun 7, 2023 •
edited
Loading

@ngreenwald @cliu72 This is good to go.

@camisowers @alex-l-kong Added the fix in for watershed's argument dtype.

ngreenwald
ngreenwald approved these changes Jun 7, 2023
View reviewed changes
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Looks great! Big upgrades

@srivarra srivarra added this pull request to the merge queue Jun 9, 2023
Merged via the queue into main with commit 5acbee2 Jun 9, 2023
@srivarra srivarra deleted the github_actions/May_updates branch June 9, 2023 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ngreenwald ngreenwald ngreenwald approved these changes

@alex-l-kong alex-l-kong alex-l-kong approved these changes

@camisowers camisowers camisowers approved these changes

@cliu72 cliu72 Awaiting requested review from cliu72

Assignees

@srivarra srivarra

Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Document supported python versions, expand versions Unpin ipywidgets in the future Incorporate cell execution time
5 participants
@srivarra @cliu72 @ngreenwald @alex-l-kong @camisowers