Skip to content

build: move to the official pysepal 4.1.0 (drop the SerafiniJose fork) - #54

Open
SerafiniJose wants to merge 21 commits into
mainfrom
feat/pysepal-v4
Open

SerafiniJose wants to merge 21 commits into
mainfrom
feat/pysepal-v4

Conversation

@SerafiniJose

Copy link
Copy Markdown
Collaborator

Summary

Runs the app on the PyPI release pysepal==4.1.0 instead of SerafiniJose/pysepal@spatial-risk. Each fork feature the app relied on is now upstream under a different API, or handled in the app:

Fork feature Now
AOI restore from value= + AoiResult.asset AoiSpec saved in the manifest and restored through AoiView(spec=); older manifests are healed on load
Browser locale (LocaleState) pysepal.i18n.current_locale(); MapApp(locales=) builds the selector
Voila local sessions pysepal 4 session topology (SEPAL=true gives a process session)
SOLARA_TEST dev login PYSEPAL_DEV_AUTH=1 plus prime_dev_auth()
Theme toggle wiring owned by pysepal
FileInput base_path on SEPAL gui/scripts/picker_paths.resolve_picked_path at the four picker sites
Admin/asset picker initial= v4 value= / codes=
Download pill progress detail falls back to the stock spinner; comes back with no app change once openforis/pysepal#1074 is merged

Dependencies: pysepal==4.1.0, solara>=1.60.3,<2 (pip; conda-forge stops at 1.58.2), ee-client>=3.2.1,<4. jupyter-loopback[comm]==0.3.3 is unchanged.

Testing

  • Full suite on pysepal 4.1.0: 2662 passed, 7 skipped. test_wall_clock_does_not_regress failed once while the app and a browser were running on the same machine, and passes alone.
  • Local browser: dev login, project load, ADMIN AOI restored, Earth Engine layer drawn.
  • SEPAL (sepal.io sandbox, a real voila kernel started by the platform's jupyter server, pysepal 4.1.0): SEPAL=true in the kernel, session plan PROCESS, SepalClient built from the sandbox key (user files listed), an Earth Engine call returns, tile URLs go through /api/sandbox/jupyter/proxy/{port}.

Deploy

Rebuild the SEPAL env from sepal_environment.yml: pysepal moves from the fork to PyPI, and solara is now installed by pip. In a local .env, replace SOLARA_TEST=true with PYSEPAL_DEV_AUTH=1.

Pin pysepal==4.0.0 in sepal_environment.yml and pyproject.toml, replacing
the archived SerafiniJose/pysepal spatial-risk fork (tag
spatial-risk-v3-final). Bump ee-client to >=3.2.1,<4 (v4's floor: the
assets cache survives a cancelled fetch) and move solara to the pip
section pinned >=1.60.3,<2 (v4's floor; conda-forge tops out at 1.58.2,
so a conda entry would be shadowed anyway).
Docstring lint debt (D103/D205/D209) that the pre-commit ruff hook reports on the whole file; no behaviour change.
pysepal 4 dropped LocaleState/resolve_locale_state. gui.i18n now reads
pysepal.i18n.current_locale(), and a use_app_locale() hook in Page swaps
the Translator-backed t() when the locale changes. MapApp.element mounts
pysepal's own language selector from locales=app_available_locales(), so
the fork's LocaleSelect memo and bind_locale_state wiring are gone.
Migrate v4 dev-auth flag: SOLARA_TEST -> PYSEPAL_DEV_AUTH with guarded
prime_dev_auth() call. Module-level _DEV_AUTH_ARMED reused by seed helpers
to avoid repetition; conftest.py guards test isolation; text-level tests
verify flag presence and guard correctness.
…_dev_auth comment

Finding 0: The test test_solara_test_flag_removed() contained "SOLARA_TEST"
in its docstring and assertion, causing the grep to find itself. Build the
needle without the literal using string concatenation and rephrase the docstring
to "The legacy dev flag is gone; PYSEPAL_DEV_AUTH replaces it."

Finding 1: The comment above prime_dev_auth() contained inaccurate claims
about RuntimeError vs ValueError and what prevents HTTP login from the render
path. Reword to correctly state that prime_dev_auth() raises RuntimeError when
PYSEPAL_DEV_AUTH is not armed (not a no-op), and ValueError if credentials
are unset when armed. The guard is necessary because the function is not
a no-op.

Both fixes verify against the task requirements:
- grep -rn SOLARA_TEST ... returns empty output
- All 21 tests in test_app_imports.py pass
pysepal 4's use_notifications(required=True) raises
NotificationProviderError at render when no NotificationProvider is
mounted, where the fork silently handed back a NoopNotifier. Add
tests/test_notifications_provider_contract.py to pin that against
VariablesTile, plus a shared tests/_notification_host.py render helper
(NotificationHost / render_under_notifications) used by the 11 existing
modules whose renders assumed the fork's NoopNotifier fallback:
test_allocation_edit_failed, test_derived_delete_files_wiring,
test_download_overwrite_wiring, test_inference_map_layers,
test_postprocess_tile_threading, test_process_tile_hint,
test_process_tile_pure_status, test_process_tile_reference_strip,
test_toolbox_tile, test_variable_delete_files_wiring,
test_variables_tile_edit_reharmonizes.

Add an autouse backstop in conftest.py that drains any NotificationBus
left registered at PROCESS scope by an unclosed render context, so a
missed rc.close() cannot silently mask the contract for a later,
provider-less test.

Skip the 4 test_notify_bridge.py tests asserting TrackedTask.progress_detail
(a fork-only field upstream pysepal 4 does not carry yet) instead of
letting them fail, so they resume automatically once that field lands.

Reword the stale "resolves a real Notifier instead of a first-render
NoopNotifier" comment in gui/solara_app.py and the matching
test_app_imports.py docstring to describe the v4 raise.
…h_a_project

Render via render_under_notifications with handle_error=False instead of
a bare reacton.render with the default handle_error=True, which swallowed
the NotificationProviderError into an error placeholder widget while the
lone assertion (box is not None) stayed vacuously true. Also close the
render context and assert the tool-list text renders.
Page() no longer builds a ThemeToggle widget or mirrors it into
solara.lab.theme via a use_effect: v4's ThemeToggle already syncs a
scope-keyed ThemeState, and every use_theme_dark()/use_dark_effective()
reader already follows that state. Share one get_current_theme_state()
instance with SepalMap and MapApp.element via theme_state= instead
(SepalMap(theme_toggle=...) is deprecated in v4; MapApp builds its own
toggle bound to the given theme_state when none is passed).

Adds a source-level guard in test_app_imports.py since tests/test_theme*
does not exist.
The manifest's AOI metadata now carries "aoi_spec" (AoiSpec.to_dict(),
written by _aoi_metadata so manual save and selection-time attach agree),
plus the legacy "asset" dict for ASSET selections for one release.
load_aoi keeps its signature and returns the AoiResult with spec= (v4 has
no asset= field); pre-spec manifests get a synthesized spec (DRAW from the
sidecar, ASSET from the legacy dict, ADMIN via the new admin_code_chain
helper), and an unreadable spec falls back to that instead of failing
the load.

AoiTile drops the keyed remount: the picker gets spec=app_state.aoi_spec
and a clear_ref, do_load writes the restored spec after the signal bump,
and the tile resets the picker on a switch AoiView would not act on
(no spec, or the same non-DRAW spec again), redrawing the kept AOI that
the reset removes from the map.
…op fork-only initial=)

pysepal 4 has no `initial=` on AdminLevelSelector or AssetSelectComponent,
so both pickers raised TypeError at render.

The admin cascade is now restored through `codes`. The chain comes from
aoi_io.admin_code_chain and is read once, when the cascade mounts.
User picks are published from on_codes. The cascade's own None is
dropped, so a chain that cannot be read no longer wipes a prefilled
selection.

The GEE asset field is seeded through `value` from a memo taken at
mount. The modal's id is no longer echoed back, so the selector's
draft is not replaced after each pick.
pysepal 4.0's FileInput lists remote files through the user-files API,
whose entries are relative to /home/sepal-user, and leaves base_path
empty, so a picker fed a sepal_client returns e.g. downloads/x.tif —
which resolves against the read-only app CWD on SEPAL instead of $HOME.

Add gui/scripts/picker_paths.resolve_picked_path and route the four
FileInputComponent sites (variable_modal, prediction_form_dialog,
borders_picker, allocation_form) through it before the value reaches
state/pydantic. Remove once upstream PR-2 seeds base_path.
Reword the notify_bridge docstrings to make stock pysepal 4.0 the
baseline and the detail= alternating-progress path the pysepal PR-1
forward path; note NoopNotifier's WARNING log for a job run with no
provider mounted. Fix leftover 'session-scoped'/'module-level fallback'
theme wording (3.x's process-global ThemeState) in evaluation_results.py,
echarts.py and test_evaluation.py to match v4's kernel-scoped ThemeState.
Adds openforis/pysepal as a project marketplace and enables
pysepal@pysepal, so the plugin's pysepal/pysepal-app skills load
alongside the project's own pysepal-app-architecture skill.
gpd.read_file parses an ISO-date-like sidecar property (one a QGIS edit
adds) into a Timestamp, and the legacy DRAW spec synthesis serialized it
with a bare gdf.to_json(), raising "Object of type Timestamp is not JSON
serializable". _restore_spec called that synthesis unguarded, so load_aoi
raised and the whole project failed to open; the newer-schema fallback
took the same path.

Serialize the drawing with to_json(default=str) so the picker keeps it,
and guard the legacy synthesis in _restore_spec so any failure there
loads the AOI with a blank picker instead of failing the load.
…chive

The pin comments said the SerafiniJose fork is archived under tag
spatial-risk-v3-final. Neither is true: the tag is not pushed, and the
fork stays live while it hosts the upstream PR branches. Say what the
pin replaces instead.
PipelineHeader read solara.lab.use_dark_effective(), which nothing
feeds since 2b32b93 removed the ThemeState mirror, so the badge, the
current-step ring and the filled segments stayed on the dark primary
in every theme. Read pysepal.solara.use_theme_dark() instead, same as
evaluation_results.py and model_stats_panel.py.

Also fixes pre-existing docstring lint debt in test_pipeline_header.py
that blocked the ruff pre-commit hook on this file.
do_load restores the AOI inside app_state.restoring_project(), so the
attach-on-select effect is refused while that window is open, by
design. For ADMIN/ASSET methods the autoselect re-run publishes an
AoiResult that compares equal to the one just restored, so solara's
equals_extra skip means aoi_result never changes afterwards and the
effect never re-fires on its own — the legacy manifest keeps aoi_spec
null until some later manual save. Call attach_current_aoi once right
after the restore window closes; it is idempotent, so an already
current manifest is left untouched, and a failed heal is logged and
swallowed rather than turned into a load error.
Scope do_load's heal call to manifests with no aoi_spec at all (key
missing or null). attach_aoi rewrites anything that differs from what
this app version would write, so calling it unconditionally would
downgrade a manifest saved by a newer app version (a spec
schema_version/fields this one can't read) back to a legacy-synthesized
one. Pinned with a test that a future-version aoi_spec survives a load
byte-identical.

Also, from review: make the idempotency test exact by counting
Project.save calls instead of comparing mtimes; assert the failed-heal
warning actually names the project (caplog); assert the header badge
colour too, not just the ring, in the G1 theme test and drop its dead
kwargs/redundant state.dark set; fix a stale "below" in a do_load
comment; fold the repeated pygaul fakes into one fixture and drop a
redundant per-module translator warm-up already covered by conftest.
Comment thread gui/scripts/aoi_io.py
_DIGEST_GRID_SIZE = 1e-6


def admin_code_chain(admin_code: Any, level: int) -> Optional[Tuple[str, ...]]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is this required?

from pathlib import Path


def resolve_picked_path(value: str | None, sepal_client) -> Path | None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FileInput returns absolute workspace paths with the latest pysepal #1078 and #1079 (already merged). I tested the SEPAL picker files and loaded the selections on the map. Once we do a release and we pin it here, we can remove picker_paths.py and use Path(value) at its four call sites.

Comment thread gui/scripts/aoi_io.py
return True


def _legacy_spec(metadata: Dict[str, Any], gdf: Any) -> Optional[Any]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this looks like a backwards compatibility shim, for users that created projects with previous schemas, can you confirm if this is actually needed? this might be related to the previous comment

Comment thread gui/tile/aoi_tile.py
clear_ref = solara.use_ref(None)
last_signal = solara.use_ref(restore_signal)

def reset_picker_on_switch():

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this logic seems complex, isn't this something we already support in pysepal?

I read the comments quickly but it might be something that can be also affecting other pysepal apps?

primary = (
themes.dark.primary if solara.lab.use_dark_effective() else themes.light.primary
)
primary = themes.dark.primary if dark else themes.light.primary

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is this needed? anything missing from pysepal implementation?

Comment thread gui/solara_app.py
# is not armed (it is not a no-op), hence the guard; when armed it raises
# ValueError if the LOCAL_SEPAL_* credentials are unset.
if _DEV_AUTH_ARMED:
prime_dev_auth()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is this needed? what is the use case it was made for?

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