Skip to content

Conversation

@cbrnr
Copy link
Contributor

@cbrnr cbrnr commented Feb 2, 2024

Fixes #12387.

@cbrnr cbrnr changed the title Remove jinja2 and pooch from sys_info Remove jinja2 and pooch from sys_info Feb 2, 2024
Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Works for me. I think in principle maybe we could want all core dep versions listed somewhere, but in practice over the many years we've had this I don't think these have ever been useful to have reported. So I'm okay with removing them in favor of trimming things down a bit. But @drammock you had suggested possibly moving them rather than removing them so I'll let you look before merge

@cbrnr
Copy link
Contributor Author

cbrnr commented Feb 2, 2024

I'm also in favor of trimming down the output to things that will be relevant for us. It is unlikely that a bug is caused by pooch or jinja2 at the end of the day. Of course, we already have a dependencies="developer" option, where we could also move these packages to.

@larsoner
Copy link
Member

larsoner commented Feb 2, 2024

Of course, we already have a dependencies="developer" option, where we could also move these packages to.

Works for me and might satisfy @drammock's desire to keep them somewhere (he had suggested misc I think but maybe lumping under developer is good enough)

@drammock
Copy link
Member

drammock commented Feb 2, 2024

including under developer sounds ideal to me. IIRC there was a brief period when pooch version mattered, which could in theory happen again (e.g., if they decided to do a major API change)

Copy link
Contributor Author

@cbrnr cbrnr left a comment

Choose a reason for hiding this comment

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

I added the other required packages to a new section called "Infrastructure". Well, all but lazy_loader, which doesn't have a version attribute.

@drammock
Copy link
Member

drammock commented Feb 2, 2024

pushed a commit to hopefully fix Circle, which seems to have been broken by docdict changes in #12195

cc @withmywoessner; see 43094a7; I think the only necessary changes were the ones relating to blank lines, but I took the opportunity to clean up the formatting a bit too

@drammock drammock enabled auto-merge (squash) February 2, 2024 17:45
@drammock
Copy link
Member

drammock commented Feb 2, 2024

reported the missing lazy_loader version upstream: scientific-python/lazy-loader#96

@drammock
Copy link
Member

drammock commented Feb 2, 2024

pip-pre failure is being tracked in #12413

@larsoner larsoner disabled auto-merge February 2, 2024 19:38
@larsoner larsoner merged commit 57611ae into mne-tools:main Feb 2, 2024
@larsoner
Copy link
Member

larsoner commented Feb 2, 2024

Thanks @cbrnr !

@cbrnr cbrnr deleted the sysinfo branch March 12, 2024 10:08
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
Co-authored-by: Daniel McCloy <dan@mccloy.info>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider removing jinja2 from mne.sys_info()

4 participants