Skip to content

Conversation

@tangkong
Copy link
Contributor

@tangkong tangkong commented Jan 14, 2025

Description

Motivation and Context

We want to defer preset loading for performance reasons.

This will fail tests until the corresponding pcdsdevices PR is merged.

How Has This Been Tested?

I was using this to test presets while developing pcdshub/pcdsdevices#1317

Where Has This Been Documented?

This PR

Left: Before, Right: After
image

In getting that screenshot I ran into a variety of load times, preset deferral was always better but by varying margins.

Ran on a clone of XCS's hutch-python profile, copied into my dev area

Pre-merge checklist

  • Code works interactively (a real hutch config file can be loaded)
  • Code contains descriptive docstrings, including context and API
  • New/changed functions and methods are covered in the test suite where possible
  • Test suite passes locally
  • Test suite passes on GitHub Actions
  • Ran docs/pre-release-notes.sh and created a pre-release documentation page
  • Pre-release docs include context, functional descriptions, and contributors as appropriate

@tangkong tangkong force-pushed the perf_defer_preset_loading branch from 90915dc to 8c22e1a Compare January 15, 2025 01:27
@tangkong tangkong marked this pull request as ready for review January 15, 2025 17:14
@tangkong
Copy link
Contributor Author

I guess there are no real tests to do here, since this is a pcdsdevices feature.

@ZLLentz
Copy link
Member

ZLLentz commented Jan 15, 2025

Does this implicitly change our minimum supported pcdsdevices version? Is this something we would address in the conda forge feedstock?

@tangkong
Copy link
Contributor Author

tangkong commented Jan 15, 2025

Ah you're right. That slipped my mind.

This will technically rely on a pcdsdevices version that doesn't exist yet. I'm reluctant to tag pcdsdevices right now because we might accumulate more changes, but once I change the pins the tests will fail. I'll move this back to WIP until we're ready to more forward.

More incentive for me to look at pyca / typhos I guess

@tangkong tangkong marked this pull request as draft January 15, 2025 17:37
@ZLLentz
Copy link
Member

ZLLentz commented Feb 14, 2025

I found this again in my PR reviews backlog. Maybe we should tag pcdsdevices next week.

@tangkong
Copy link
Contributor Author

Ah yes. This also reminds me of the whole tagging/new-conda-effort we had going a month ago

@tangkong tangkong force-pushed the perf_defer_preset_loading branch from 8c22e1a to 8bfdf0f Compare March 17, 2025 01:25
@tangkong
Copy link
Contributor Author

tangkong commented Mar 17, 2025

This will fail until we've tagged/deployed pcdsdevices, I've updated the pin here slightly prematurely

@tangkong tangkong force-pushed the perf_defer_preset_loading branch from 3e499d9 to 17d87f7 Compare April 2, 2025 18:20
@tangkong
Copy link
Contributor Author

tangkong commented Apr 2, 2025

Apparently in py3.12 flake8 started linting strings, a problem that was fixed with 6.1.0. I suspect that shifting to ubuntu latest shifted the default python version to 3.12, exposing us to this error.

I verified that flake8 v6.0.0 and py3.12 reproduces this

Updating the flake8 version in the pre-commit config fixes this

@tangkong tangkong marked this pull request as ready for review April 2, 2025 18:54
@tangkong
Copy link
Contributor Author

tangkong commented Apr 2, 2025

This is good to go now, as long as we're sure that we're ok with ipython being unpinned

Copy link
Member

@ZLLentz ZLLentz left a comment

Choose a reason for hiding this comment

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

I'm happy with this

@tangkong tangkong merged commit 234efeb into pcdshub:master Apr 7, 2025
11 checks passed
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