Skip to content

Add basic structure for asv performance measurement#289

Merged
LucaMarconato merged 7 commits intoscverse:mainfrom
Czaki:add_asv
Aug 16, 2024
Merged

Add basic structure for asv performance measurement#289
LucaMarconato merged 7 commits intoscverse:mainfrom
Czaki:add_asv

Conversation

@Czaki
Copy link
Copy Markdown
Collaborator

@Czaki Czaki commented Aug 14, 2024

Based on napari asv (https://asv.readthedocs.io/en/v0.6.1/) configuration, I have created initial configuration for this project.

Added benchmarks are only for adding structure. After review of the code, I'm not sure where I should start with benchmarks.

@LucaMarconato
Copy link
Copy Markdown
Member

Ah no the problem is not with mamba, but that we need to run asv in main first. I'll revert my commit.

@Czaki
Copy link
Copy Markdown
Collaborator Author

Czaki commented Aug 15, 2024

Ah no the problem is not with mamba, but that we need to run asv in main first. I'll revert my commit.

No problem is with lack of definition of release commit (it point to main, instead of commit hash).

When you plan to do next release? I will create separate PR to improve release process and fix lack of this information.

@LucaMarconato
Copy link
Copy Markdown
Member

@Czaki I checked the code and it looks great to me! I have noticed that in a few places there seems to be some redundancy in some configuration that could lead to problems in the future if we are not careful when making changes.

For instance I see that Python 3.11 is specified manually in asv.conf.json, benchmarks.yml. I could imagine that forgetting to change all of them at the same time could lead to subtle errors. Instead of "centralizing" this information, I would suggest to address this by writing in this PR a explanation/checklist (it can be very small, like 5-10 lines of text) of how to use asv for our dev purposes and what to do when something needs to be changed.

Example checklist on how to add/edit benchmarks:

  • remember to split qt and non-qt tests
  • before committing, test locally with asv ...
  • if you change the Pyhton version, change it in asv.conf.json and benchmarks.yml

WDYT?

@LucaMarconato
Copy link
Copy Markdown
Member

LucaMarconato commented Aug 15, 2024

Probably we will be doing a release of spatialdata and spatialdata-plot within 2 weeks. In napari-spatialdata we could make it if needed, also today.

@Czaki
Copy link
Copy Markdown
Collaborator Author

Czaki commented Aug 15, 2024

Could you check readme now?

@LucaMarconato
Copy link
Copy Markdown
Member

Looks great, thanks!

@LucaMarconato LucaMarconato merged commit 13e69df into scverse:main Aug 16, 2024
@Czaki Czaki deleted the add_asv branch August 16, 2024 16:18
@Czaki Czaki mentioned this pull request Aug 18, 2024
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