-
Notifications
You must be signed in to change notification settings - Fork 14
Feature/concrete-data-collector #161
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.
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
Feature/concrete-data-collector #161
Conversation
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #161 +/- ##
==========================================
+ Coverage 89.88% 91.82% +1.93%
==========================================
Files 12 13 +1
Lines 1780 1920 +140
==========================================
+ Hits 1600 1763 +163
+ Misses 180 157 -23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…en-geo/mesa-frames into feature/concrete-datacollector
for more information, see https://pre-commit.ci
…en-geo/mesa-frames into feature/concrete-datacollector
…ature/concrete-datacollector
for more information, see https://pre-commit.ci
…en-geo/mesa-frames into feature/concrete-datacollector
…en-geo/mesa-frames into feature/concrete-datacollector
32914bc
to
a2bc8e8
Compare
for more information, see https://pre-commit.ci
Hey @adamamer20 - I have finished the test for postgress |
Hey @Ben-geo, Given that, I think the simplest and cleanest solution is to run the Postgres tests only on Linux and macOS, and skip them on Windows to avoid the long setup with Chocolatey. We can implement this by:
env:
SKIP_PG_TESTS: ${{ runner.os == 'Windows' }}
import pytest
import os
@pytest.mark.skipif(
os.getenv("SKIP_PG_TESTS") == "true",
reason="PostgreSQL tests are skipped on Windows runners"
)
def test_postgress(...):
... This tradeoff is worth it, we get full DB coverage on Linux/macOS, and significantly faster CI runs on Windows without the extra complexity. |
…en-geo/mesa-frames into feature/concrete-datacollector
Yup agreed this is what I had thought we should do too !!! Okie that's it I think with that everything is done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Glad to merge this :)
@coderabbitai review |
✅ Actions performedReview triggered.
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ModelDF
participant DataCollector
participant StorageBackend
User->>ModelDF: Run simulation step(s)
ModelDF->>DataCollector: Call _collect() at step
DataCollector->>ModelDF: Retrieve model/agent reporters
DataCollector->>DataCollector: Store data in memory (LazyFrame)
User->>DataCollector: Call _flush() (optional)
DataCollector->>StorageBackend: Write data (memory/CSV/Parquet/S3/PostgreSQL)
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores