Experiment mwp

[henrikjacobsenfys]

Add experiment class and minimal functionality

[codecov]

Codecov Report

❌ Patch coverage is 97.29730% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (develop@fef02f4). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/easydynamics/experiment/experiment.py	97.26%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             develop      #90   +/-   ##
==========================================
  Coverage           ?   97.63%           
==========================================
  Files              ?       30           
  Lines              ?     1606           
  Branches           ?      268           
==========================================
  Hits               ?     1568           
  Misses             ?       21           
  Partials           ?       17           

Flag	Coverage Δ
integration	0.00% <0.00%> (?)
unittests	97.63% <97.29%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AndrewSazonov]

Small suggestion: it would be nice to update the placeholder markdown header cells to use the notebook title and short description I added in the previous PR (and also add one to the new experiment.ipynb). That way the code cells can automatically stretch to the full page width.

[rozyczko]

some minor issues raised

[rozyczko]

You're modifying dimensions in-place, while iterating over its items. This can lead to BAD things. Consider creating a copy of dimensions before and iterating over the copy (or the other way around)

[henrikjacobsenfys]

Thanks!

[rozyczko]

None is not the advertised sc.Variable. Consider changing -> sc.Variable to sc.Variable | None

[rozyczko]

Move the import to the top

[henrikjacobsenfys]

Top of the file or top of the function? I believe I put it here to only do the import if we're actually saving data, so after input checking :)
Edit: put it at the top of the file

[rozyczko]

Shouldn't the argument be display_name?

[henrikjacobsenfys]

It should indeed, I wonder how it passed the test

[rozyczko]

This should read experiment.ipynb

[henrikjacobsenfys]

Why can't I react with the 'see no evil' emoji?

[rozyczko]

remove commented out code

[rozyczko]

this method should probably be typed to return -> bool

[henrikjacobsenfys]

Fixed this and another missing type hint :)

[henrikjacobsenfys]

Small suggestion: it would be nice to update the placeholder markdown header cells to use the notebook title and short description I added in the previous PR (and also add one to the new experiment.ipynb). That way the code cells can automatically stretch to the full page width.

Good point, will do this!

[rozyczko]

Please reset the notebooks

[rozyczko]

Clean up the notebooks!!!

[henrikjacobsenfys]

Ok, I just thought it might be nice in the tutorials to see the expected output. I at least appreciate it when I can see what kind of figures are produced by a piece of code in a tutorial.
Is it better to add screenshots instead?

[rozyczko]

hmm maybe... it does make sense to see the complete notebooks from the get go. The users can always clean them up before running.
However, having all the charts, pictures and metadata is prone to constant updates, which inflate the size of each commit...
I think the general consensus is to keep notebooks clean but we might discuss it.