text:
Cc/publish alert

Pre-Flight checklist

[x] Testing

[x] Code is tested
[x] Changes have been manually tested

What are the relevant tickets?
Finishes #991
What's this PR do?
A few things:

adds a custom alert prompt instead of window.confirm, see screenshot below
control opening the publishing drawer via a search param ?publish=. Its value is irrelevant. Its key present means "drawer is open".
Use the ?publish= search param along with react-router's prompt component to prompt user for confirmation when publishing with a dirty form (this was the ticket goal ✅)
Adds a new frontend integration test helper to live alongside the old. See code comment.

How should this be manually tested?

Restart docker to pick up new dependencies. Possibly only need to restart watch: docker-compose restart watch.
Try publishing with unsaved changes on the metadata page (or menu page; any page with editable content where publish button is still visible). You should be prompted for confirmation if and only if you have unsaved changes.
You should still be prompted when navigating to other routes with a dirty for
You should not be prompted in other scenarios.

Screenshots (if appropriate)

Codecov Report

Merging #1274 (93a4346) into master (0d21a94) will increase coverage by 4.89%.
The diff coverage is 86.39%.

@@            Coverage Diff             @@
##           master    #1274      +/-   ##
==========================================
+ Coverage   89.48%   94.37%   +4.89%
==========================================
  Files         220      104     -116
  Lines        9174     3220    -5954
  Branches     1730      777     -953
==========================================
- Hits         8209     3039    -5170
+ Misses        847      174     -673
+ Partials      118        7     -111

Impacted Files
Coverage Δ

static/js/components/BasicModal.tsx
100.00% <ø> (ø)

static/js/components/util/Prompt.tsx
86.66% <66.66%> (ø)

static/js/components/Header.tsx
90.69% <73.33%> (-0.97%)
:arrow_down:

static/js/util/IntegrationTestHelper.tsx
77.08% <77.08%> (ø)

static/js/util/factories/websites.ts
98.52% <85.71%> (+0.04%)
:arrow_up:

static/js/test_util.ts
94.11% <89.47%> (-5.89%)
:arrow_down:

...tatic/js/components/util/ConfirmDiscardChanges.tsx
92.85% <92.30%> (-7.15%)
:arrow_down:

...tic/js/components/util/PromptConfirmationModal.tsx
100.00% <100.00%> (ø)

static/js/components/util/Router.tsx
100.00% <100.00%> (ø)

static/js/hooks/search.ts
100.00% <100.00%> (ø)

... and 120 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d21a94...93a4346. Read the comment docs.

@gumaerc As discussed, this probably should have been at least two PRs. Possibly three:
These changes make the modal pretty instead of window.prompt

Router.tsx and its test file
PromptConfirmationModal.tsx

These changes make the location change on publish, which triggers the prompt:

Header.tsx
ConfirmDiscardChanges.tsx has some meaningful related changes

Additionally:

IntegrationTestHelper.tsx is the new test helper
Header.test.tsx was completely re-written using the new helper

#1275 is tracking this.
Minor clarification:

some...through Sinon and some running through the React testing library.
We have two inconsistencies right now:

Enzyme vs React Testing Library (RTL)
Sinon vs Jest...We use Jest for test running, assertions, and mocking. Except mocking API calls. We use Sinon for mocking API calls.

As we switch from Enzyme to RTL, we'll should switch to Jest exclusively for mocking.

authors: ChristopherChudzicki codecov-commenter

repo: mitodl/ocw-studio


text:
Create Heroku app and infrastructure for MIT Open

User Story

As a developer working on MIT Open I want to be able to deploy my changes to a shared environment for testing and validation
As an MIT Open product owner/stakeholder I want to be able to review and test features in the application

Description/Context
We are deploying a new copy of MIT Open/Open Discussions to facilitate development of the new product vision. To that end we need a Heroku application and associated AWS infrastructure set up that mimics the current open-discussions environment.
Acceptance Criteria

[x] There is a Heroku application pipeline configured to deploy from https://github.com/mitodl/mit-open
[ ] There is a Salt proxy configured to manage settings for the Heroku pipeline
[ ] There is a Pulumi project for managing the AWS infrastructure needed by MIT Open
[x] This also includes the OpenSearch cluster so that we can keep the two runtimes completely separate.

Was gonna start populating vault values, however the mount hasn't been created. Looked at this PR https://github.com/mitodl/ol-infrastructure/commit/4b8f1510c3c5230207688ce16dd20d53688a9965 but just wasn't sure where to start and didn't want to override any previous work. Will work on getting Touchstone working with Keycloak.

We need to decide on a domain name to use for the Heroku managed TLS cert and we also need to setup a new mailgun domain

@rhysyngsun We added you as a collaborator to the mitopen-rc app in Heroku. I believe that salt-ops code wants very badly to update the config vars for that app but it cannot because the build is failing with:

-----> Generating cache tables
       Traceback (most recent call last):
         File "/app/manage.py", line 12, in <module>
           from django.core.management import execute_from_command_line
       ModuleNotFoundError: No module named 'django'

I reconciled the buildpacks vs. odl-open-dicussions-rc and those look good but I noticed that the detected "frameworks" for mitopen-rc is just Node.js while there are a bunch listed for odl-open-discussions-rc. I'm not sure where those come from? app.json.
Let me know if you want to get together at some point and get it further along.

For Mailgun, I discussed with Peter and we're just going to use the existing domain that's applied to discussions, at least for now. @shaidar

@Ardiea did you solve this? looks like it's deployed recently

source: gharchive/issue

authors: Ardiea blarghmatey rhysyngsun shaidar

repo: mitodl/ol-infrastructure

license_type: permissive

license_source: github-api


text:
Create an index of available ATT&CK data

As a user of the ATT&CK website, I want to be able to easily access and download ATT&CK data in various formats.
~As we introduce additional ways to access raw data (CSV, Layers, etc) from the ATT&CK website, we should create an index page that allows for easy searching and downloading of data. Users may want to download all of the "group technique usage layers" at the same time instead of having to go to each group page simultaneously.~
~- We should create an interface which allows the user to search/filter for data to download by name and type.~
~- We should allow users to download individual files (E.g a single layer, CSV) from this interface.~
~- We should allow users to download bundles of all data of a certain type (e.g all group technique usage layers as a .zip or .tar.gz file).~
We should provide a listing of the various interfaces for working with ATT&CK on the working with ATT&CK resources page. This should provide links to the STIX and Excel data as well as a listing of useful tools for working with the dataset.

See branch feature/#142-working-with-attack-refresh

repo: mitre-attack/attack-website


text:
Add dummy plugin and CLI to execute for testing

This PR introduces a dummy plugin rand_data that takes an int N and returns a vec of random bytes with length N. It also adds a CLI subcommand hc plugin that will do the work of starting, intializing, and querying the plugin for an array of bytes, then closing. This is a temporary and hidden subcommand to help us with testing the plugin query-protocol code we write.

@j-lanson just needs a rebase

source: gharchive/pull-request

authors: alilleybrinker j-lanson

repo: mitre/hipcheck


text:
glob!: Add three-arg version to specify base_path

The new three-arg glob! allows users to specify the base directory the glob is working against. This works around the limitation that blobs like ../foo/*.json do not resolve with the current walkdir + globset implementation.
This allows the usage of glob! in nested situations where the globbed files are in a parent directory:
src
├── main.rs
├── snapshots
│   └── insta_glob_test__insta_glob_dir.snap
├── submod
│   └── mod.rs -- glob! tests are here
└── test_data
    └── foo.json

glob!("../test_data", "*.json", |path| { ... })

Note that this implementation could provide a nicer interface where glob! or glob_exec would accept a glob like ../test_data/*.json. However, implementation of this isn't as easy as I hoped as we would need to detect & split the path elements from the glob elements (i.E. ../test_data/*.json -> ("../test_data/", "*.json")).
I can tackle this if you prefer this simpler interface.

This looks good to me. Thank you!

authors: mitsuhiko the-kenny

repo: mitsuhiko/insta


text:
TypeError: a bytes-like object is required, not 'str'

python-geoip (1.2)
python 3.6.9
Reproduce:
from geoip import geolite2
match = geolite2.lookup('8.8.8.8')

Output:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.6/dist-packages/geoip.py", line 364, in lookup
    return self._get_actual_db().lookup(ip_addr)
  File "/usr/local/lib/python3.6/dist-packages/geoip.py", line 350, in _get_actual_db
    rv = self._load_database()
  File "/usr/local/lib/python3.6/dist-packages/geoip.py", line 342, in _load_database
    return mod.loader(self, sys.modules[__name__])
  File "/usr/local/lib/python3.6/dist-packages/_geoip_geolite2/__init__.py", line 9, in loader
    return mod.open_database(filename)
  File "/usr/local/lib/python3.6/dist-packages/geoip.py", line 508, in open_database
    md = _read_mmdb_metadata(buf)
  File "/usr/local/lib/python3.6/dist-packages/geoip.py", line 382, in _read_mmdb_metadata
    buf.size() - MMDB_METADATA_BLOCK_MAX_SIZE)
TypeError: a bytes-like object is required, not 'str'

The same issue happens using a self-provided latest DB:
from geoip import open_database
db = open_database('GeoLite2-City.mmdb')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.6/dist-packages/geoip.py", line 508, in open_database
    md = _read_mmdb_metadata(buf)
  File "/usr/local/lib/python3.6/dist-packages/geoip.py", line 382, in _read_mmdb_metadata
    buf.size() - MMDB_METADATA_BLOCK_MAX_SIZE)
TypeError: a bytes-like object is required, not 'str'

Repro'd with every version of python-geoip-geolite2 available and quite a few versions of python-geoip. This has been broken for a very long time.

authors: Artanicus nic-hartley

repo: mitsuhiko/python-geoip

license_source: bigquery


text:
Fulltext search in docs

Comment from @gskyarts
Search :: The current search seems to look through navigation rather than document content, which isn't very helpful for me.

Idea: Use default shortcut to open/focus fulltext search.

repo: mittwald/flow

license: MIT


text:
fix(sponsors): replace placeholder for sponsor name by actual Sponsor.company

since #115

This has not been tested (my port 8080 is currently occupied by the enterprise application of my daily customer)

repo: mix-it/mixit


text:
docs(*): Build and publish docs to gh-pages

In a subsequent PR to the developer site i'll remove the java doc generation. This will make the developer site significantly easier to develop for. The developer site will then link to the gh-pages of this repo.
We should do this for interactive-java too, its coooool!
Used: https://dzone.com/articles/how-publish-maven-site-docs
To work out how to do this.

@ProbablePrime,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

It works btw: https://mixer.github.io/beam-client-java/

authors: ProbablePrime msftclas

repo: mixer/beam-client-java


text:
avoiding unnecessary try-catch

A cleaner fix for #333 than #446.

Thanks for the clean up @Coeur !

authors: Coeur patedit

repo: mixpanel/mixpanel-iphone


text:
Add MPNotification test with malformed image URL from customer issue.

This test exercises a real code path that caused a crash in v2.9.8 for a customer due to an invalid image URL. This test confirms that the move to NSURLComponents did actually resolve that issue.

lgtm

authors: samgreen yinfeiru


text:
闭环引发的栈溢出问题

首先感谢开源这个好用的demo
我在使用过程中，发现血缘高亮功能面对闭环会陷入死循环，报错：
jsPlumb: fire failed for event click : RangeError: Maximum call stack size exceeded

建议在寻找上下游的迭代过程中加个可配置的迭代次数参数 应该可以解决问题

你好XhivaW！非常感谢issue
1.关于闭环的问题，由于 jsplumb的性质是只负责渲染的表示工具不维护数据，所以我最初的设想也是闭环等问题由后端生成json时解决，jsplumb不负责处理
2.关于你的配置的迭代次数参数这个提案，我会在有空的时候想想怎么写好它！血缘高亮这个功能当时写完后其实有缺陷的，非常抱歉！

authors: XhivaW mizuhokaga


text:
OcaIDE fails to parse cppo ml files

Hi,
I'm using OcaIDE (http://www.algo-prog.info/ocaide/) for OCaml coding and recently, when trying your tool CPPO, I notice that the additional syntax added by CPPO makes OcaIDE fails to parse ml files.
For example, if I put #include "hello.ml" at the beginning of my file, OcaIDE fails to recognize '#' symbol, thus it cannot parse the source code.
Can you make your tool compatible with the original syntax of OCaml?
In my opinion, I suggest to enclose your new syntax by a special comment symbols of OCaml. For example:
(*@ #include "hello.ml" )
or just:
(#include "hello.ml" *)

OcaIDE should support toplevel directives, which can be used in ml files passed directly to ocaml:
(* foo.ml *)
#load "unix.cma";;

print_endline (Unix.getenv "HOME")

$ ocaml foo.ml
/home/martin

So I imagine it could easily support other directives, especially since it's possible to register custom directives, like Findlib does:
(* bar.ml *)
#use "topfind";;
#require "unix";;

print_endline (Unix.getenv "HOME")

$ ocaml bar.ml
/home/martin

Now the syntax of toplevel directives is more  restricted than the syntax of cppo directives, but at least simple directives such as #include "example.ml" fall within the standard syntax and I think should be supported. I'm not familiar with OcaIDE so I can't really make a recommendation on how much of cppo syntax it should or should not support.
It is pretty clear however that the base syntax of cppo is not going to change. The whole point was to be close to what the C preprocessor supports and not extend the syntax of OCaml, i.e. cppo can operate on files that do not follow a strict OCaml syntax.

Thank you for your explanation. I have another query:
Although the toplevel directive files can be passed directly to ocaml interpreter, they are not supported by OCaml compiler (ocamlc). Do you think that in the long term, it could be possible for OCaml compiler to have a built-in preprocessing feature like this?

You can ask on caml-list. As far as I know it hasn't been discussed. I can imagine several reasons for it, one being that camlp4 could provide some of that functionality. Another reason is that it's not extremely useful in OCaml as it is in C. Lastly, conditionals make it hard to guarantee that the resulting OCaml program will compile in all conditions.

Thanks for providing such details. About your last comment: "Lastly, conditionals make it hard to guarantee that the resulting OCaml program will compile in all conditions", I just wonder if this problem happens to CPPO? If not, how about considering to integrate your tool into OCaml compiler?

Actually cppo should not be modified in any way to be compatible with OcaIDE. The latter already supports preprocessing with camlp4 and can be easily adapted to support cppo as well. I have already modified OcaIDE sources and relatively happy with the result:
https://github.com/db4/OcaIDE/commit/229b191699525c2a714e7e345d183aca842e2da6
To use cppo you should include a special comment in a source file:
(* pp: cppo <optional cppo options> *)

Thanks! It's nice to hear about your development. I will try it out! :-)

authors: db4 mjambon taquangtrung

repo: mjambon/cppo


text:
Units for position and velocity output?

What are the units for the position and velocity output values when generating CSV files?

Depending on whether imperial or metric is chosen in the settings the units are either in feet or meters.


text:
Line::intersectsLine() is buggy

Hello,
Unfortunately, the Line::intersectsLine() method doesn't always work correctly. Try this test:
<?php
declare(strict_types=1);
require 'vendor/autoload.php';

use Location\Coordinate;
use Location\Line;

$line1 = new Line(
    new Coordinate(0.0, 0.0),
    new Coordinate(0.0, 2.0)
);
$line2 = new Line(
    new Coordinate(2.0, 2.0),
    new Coordinate(0.0, 10.0)
);
if ($line1->intersectsLine($line2)) {
    echo 'Line::intersectsLine() is buggy!'.PHP_EOL;
}

Added a test for this issue: https://github.com/mjaschen/phpgeo/compare/master...fix/96
@nilshoerrmann It seems that there's a bug when checking for line intersection for some edge cases – do you have an idea for a fix/workaround?

I'll have a look at this over the next days.

From a quick look at the source, in intersectsLine() we have these conditions:
// the lines are collinear or touch
if (
    in_array(self::ORIENTATION_COLLINEAR, $orientation, true)
    && (new Intersection())->intersects($this, $line, false)
) {
    return true;
}

This only works if all points are collinear but fails if this is only true for three points (as in the example). So we would either have to switch to high precision for all calculations – (new Intersection())->intersects($this, $line, true) – or we would have to add a more precise condition that distinguises between three collinear points (setting precision to true) and four collinear points (settings precision to false).

authors: mjaschen nilshoerrmann vlad-vinogradov

repo: mjaschen/phpgeo

license: mit


text:
Support non-interactive mode

Is there a way to invoke messer in a non-interactive mode? For example, I'd like to use it with Alfred app and call it like this:
messer m "Bob" hi

cool idea @joech4n , would be happy to consider PR's adding this support 👍

released as of https://github.com/mjkaufer/Messer/milestone/3. Thanks again @SUBmarinoff !!

repo: mjkaufer/Messer

license: ISC


text:
Can't use custom font except google font

I want to use a custom font in my MJML template. It's not a google font (for example, it's bentonSans family font). I try to use it in <mj-font> and use font-face on <mj-style>, but both of them didn't work! What should I do? Was it my bad? Or in the MJML, we can't use custom fonts?
Thank you.

Well mj-font let you import an external css file that declare font face to ensure that you won't have any issue on Outlook.
Beware, most of client ignore custom fonts https://www.caniemail.com/search/?s=font-face

authors: iRyusa sanazmemar

repo: mjmlio/mjml


text:
Using font-size: 0 triggers Rspamd and some ISP anti-spam

Describe the bug
I noticed that font-size:0px is triggering the anti-spam filter on Rspamd and some other ISP. For example some Sentry emails are going directly to spam because of it.
When I use MJML, my process is to remove all font-size:0px; and font-size:0; from the HTML, otherwise it will go directly to spam. In fact, ZERO_FONT will add only 1 to the spam score, but it will trigger another rule, BAYES_SPAM, that will add 5.1 to the spam score.
To Reproduce
Steps to reproduce the behavior:

Create a file with a MJML section, or any other element that is using font-size: 0px
Render it to HTML
Send the HTML with a SMTP server (I'm not using an external service)
See it going to SPAM with Rspamd for example

Expected behavior
Email not marked as SPAM.
MJML environment (please complete the following information):

OS: MacOS
MJML Version 4.7.1

Additional context
Related to #1045

No need to reopen the issue, this has been replied here https://github.com/mjmlio/mjml/issues/1045#issuecomment-372969066

@iRyusa Yes I understand, but the other issue was only reporting it from a code review point of view, not a real world example. Mine is a real world example from today. If you prefer you can reopen the other issue.
For example, I cannot receive Mailjet transaction emails, they are soft rejected. This zero font size triggers ZERO_FONT, MANY_INVISIBLE_PARTS and BAYES_SPAM.

You can just apply a inline rule to replace all occurences of them if you need so.
<mj-style inline="inline">
  *[style*="font-size:0px"] { font-size: 1px !important; }
</mj-style>

As we said, we don't want to remove it because it's a fix for some webkit based clients with inline-block. If you're not really happy with that, you can just fork those components, remove the font-size:0 and you'll be fine.

Thanks, I didn't know the existence of this inline rule.
However I'm a little bit surprised by your answer because in the previous issue, @ngarnier said it was not a problem because it was not really triggering the anti-spam and was not affecting delivery, but here I showed that it's maybe not true anymore, even for Mailjet. So I didn't expect this kind of answer, but it's ok, I'll go fork it.

Let me clarify, here's how we see it:

Pros of using font-size:0px: fixes rendering issues with common and popular webkit-based email clients
Cons of using font-size:0px: might add to the SPAM score of some anti-SPAM filters

Like previously mentioned, we're not seeing any deliverability issues due to this style, either at Mailjet or in the community, so the pros largely exceed the cons.
About Rspamd specifically, flagging an email as spam for the sole usage of this style would be largely debatable. While I'm no expert at Rspamd, from what I can read in their documentation though, Rspamd is not doing this. Per their documentation:

soft reject: temporarily delay message (this is used, for instance, to greylist or ratelimit messages)

Finally, even if this style adds some points to the SPAM score, good practices will largely compensate to make sure the email makes it to the inbox. If you encounter issues making it to the inbox, I would rather recommend using an ESP whose main job is precisely email deliverability.

Understood. FYI when Rspamd flags as Soft reject, it will frequently send it to the spam folder just after. On this rule, Rspamd mitigate the ZeroFont exploit that is used to show some content to the anti-phishing system that is reading all characters, even the one with a font-size:0, and the real content shown to the user.
For example : Micropommesoft.
Thanks again for your time!

Yep, the related exploit is totally understood, but what I'm challenging is rather the fact that this by itself should send an email to spam.
Thanks for sharing those details!

Rspamd does not solely flag an E-Mail as Spam if the ZERO_FONT rule is triggered.
However this will at +1 to your overall Score.
If your score passes a certain threshold (depending on individual configs of the mail providers) your email will be marked greylisted or even blacklisted. (iirc above 5 will greylist you | send mail to spam on default setting of rspamd)
So while not the only reason this is quite a big factor ( i.e forged sender will give you +0.3 points) and should, imho, be avoided if possible.
Please consider too that those spam detection rules are sometimes very loosely / will give you false positives.
Like some transactional mails from i.e. Amazon will have a score of 3 or even 4 (if you don't substract the standing/reputation of the IP/DKIM Whitelist which you as a small company usually don't have right away)
If you can with a reasonable amount of effort avoid a +1 on a spam score: please do

Again if this is your concern : https://github.com/mjmlio/mjml/issues/2062#issuecomment-713086135 there's a quick hack to get this done. We've never found any issue with spam score in our data related to MJML.

In fact the quick hack does not fully work, because it does not replace styles in conditionnal code

authors: delpes iRyusa julien1619 mellinger ngarnier


text:
Add the ability to use custom fields in the CSV file

The ingester should be able to add fields other than title and description. It should auto-detect additional fields in the CSV and if the corresponding field exists in the target content type, populate it.

Closed with 134830d34462fa2178a48e60a8f07ea4bbfb04ca.

license: Unlicense


text:
Add tagfile that indicates the Islandora Media Use URI of each file in bag

So that future users of the bag will know what each file was. Related to #1.

This information is available in the file added by the AddMediaJson plugin, but it might be useful to have an explicit manifest of this information.

Sample media_use_summary.tsv file:
MG_1410.JPG    http://pcdm.org/use#OriginalFile
2-Service%20File.jpg    http://pcdm.org/use#ServiceFile


text:
Define/implement postGIS SQL insert statement for CMAQ data

Need to define/implement postGIS SQL insert statement for CMAQ data : SR-ORG 29
http://spatialreference.org/ref/sr-org/29/

next must update to open and add multiple netcdf files

repo: mjstealey/exposures-api


text:
can you add a semicolon?

generateModule, generateEnum, generateCommonEnums, generateService, generateResolver.....
thank you

fixed this problem in v1.0.4, please check: https://github.com/mk668a/nestjs-prisma-graphql-crud-gen/compare/v1.0.3...v1.0.4


text:
Old API Keys for Yandex Translate Stopped Working Since 8/15

More info on this page.
I need to update the workflow to work with the new keys and the new terms of use.

For non-residents of Russia they support the old keys, but they became paid. See more info here.

For residents of Russia I should implement support for new cloud keys described here.

Should be fixed in 496b25c73c6d05efdc8fcbbb647da3c9b1df1b53.

repo: mkalygin/alfred-yandex-translate


text:
Documentation for watching source code directories

Description
The documentation (and, in fact, the README) state that:

Watch source code directories: you can tell mkdocstrings to add directories to be watched by MkDocs when serving the documentation, for auto-reload.

However, the link sends to an non-existing section of the documentation with no explanation. After some googling, I found a cached version of that documentation page that says this was deprecated in favor of the watch option of mkdocs. I suggest that either this paragraph (in the README and the doc index) be removed (since it's a mkdocs feature), or updated to direct the user to the proper reference (https://www.mkdocs.org/user-guide/configuration/#live-reloading).

You're right, I'll update it, thanks for the heads up :slightly_smiling_face:

README updated, thansk again.

authors: Epithumia pawamoy

repo: mkdocstrings/mkdocstrings


text:
Non-Primitive Return Types are Incorrectly Formatted

Describe the bug
mkdocstrings-python handler does not correctly format non-primitive return types in the generated documentation table, although this functionality remains correct for native types such as str, int, float, etc.
This is in contrast to the legacy mkdocstrings-python-legacy handler that generates the return types correctly.
To Reproduce
Have a return type of typing.List (from typing package), for example, and notice that the generated documentation table is incorrect. It will have a description of "typing.List {user-added docstrings}", and optionally also gets incorrectly treated as "Name".
Expected behavior
Return type is not repeated and not considered as name, just like in mkdocstrings-python-legacy.
Screenshots

System (please complete the following information):

mkdocstrings-python version: 0.8.3
Python version: 3.8.15
OS: Linux

Additional context
An example of the incorrectly generated documentation table can be found here.

Hey @w11wo, thanks for the nice report!
So, users previously asked that we support named return values. To support that, we had to use the same syntax as for parameters: name (type): Description.. It means that a return section with int: An integer. will be parsed as name=int and description=An integer.. You would have to wrap int with parentheses to have it parsed as the type instead of the name: (int): An integer..
With that said, I understand this is unexpected and non-standard, so my plan is to make this configurable. I'll eventually get to it :smile:

Also note that you don't need to write the type in the docstring if you add a return annotation to the signature: def func() -> int: ...

Thanks for the clear explanation @pawamoy!
I agree that it will be great to have that option configurable, I think it would be beneficial for a lot of people. Looking forward to the update 😄

This has been implemented for quite some time now, and documented here: https://mkdocstrings.github.io/griffe/docstrings/#google-options. Closing :slightly_smiling_face:

repo: mkdocstrings/python


text:
16 ensure typing and docstrings

Added tests for fastapi

Now ready to merge after changes from tuning integrated

Now ready to merge after changes from tuning integrated


text:
[Bug] chatglm4 mlc_llm shows error "TVMError: Check failed: append_length > 0 (0 vs. 0) : Append with length 0 is not allowed." during mlc_llm chat CLI

mlc-ai-nightly-cu122          0.15.dev404
mlc-llm-nightly-cu122         0.1.dev1355
transformers                  4.41.2
git clone https://huggingface.co/THUDM/glm-4-9b-chat
mlc_llm convert_weight ./dist/models/glm-4-9b-chat/ --quantization q4f16_1 -o dist/glm-4-9b-chat-MLC
mlc_llm gen_config ./dist/models/glm-4-9b-chat/ --quantization q4f16_1 --conv-template glm  -o dist/glm-4-9b-chat-MLC/
It shows
The repository for dist/models/glm-4-9b-chat contains custom code which must be executed to correctly load the model. You can inspect the repository content at https://hf.co/dist/models/glm-4-9b-chat.
You can avoid this prompt in future by passing the argument trust_remote_code=True.
Do you wish to run the custom code? [y/N] y
Special tokens have been added in the vocabulary, make sure the associated word embeddings are fine-tuned or trained.
Add trust_remote_code=True
fast_tokenizer = AutoTokenizer.from_pretrained(str(config.parent), use_fast=True, trust_remote_code=True)
It shows error
AttributeError: 'ChatGLM4Tokenizer' object has no attribute 'backend_tokenizer'
/workspace/mlc-llm/cpp/tokenizers/tokenizers.cc:154: Warning: Tokenizer info is not detected as tokenizer.json is not found. The default tokenizer info will be used.
Segmentation fault (core dumped)
mlc_chat_config.tokenizer_info = asdict(Tokenizer.detect_tokenizer_info(str(output)))  run into Segmentation fault

I'm not sure but GLM may use a customized tokenizer which is not supported yet

I'm not sure but GLM may use a customized tokenizer which is not supported yet

https://github.com/mlc-ai/mlc-llm/pull/1313 mentioned chatglm3 back, but I tried chatglm3-6b, it show same error

That is related to a recent change in tokenizer in #2416. We will fix that soon

See #2532

@Ubospica thanks!
I just tested latest package
mlc-ai-nightly-cu122          0.15.dev404
mlc-llm-nightly-cu122         0.1.dev1382
mlc_llm gen_config ./dist/models/glm-4-9b-chat/ --quantization q4f16_1 --conv-template glm  -o dist/glm-4-9b-chat-MLC/ works now
But after compilation
mlc_llm compile ./dist/glm-4-9b-chat-MLC/mlc-chat-config.json --device cuda  --quantization q4f16_1 --model-type chatglm  --output ./dist/libs/glm-4-9b-chat/glm-4-9b-chat-cuda.so
If running with cli
mlc_llm chat  ./dist/glm-4-9b-chat-MLC/ --device "cuda" --model-lib dist/libs/glm-4-9b-chat/glm-4-9b-chat-cuda.so
It shows error like below
mlc_llm chat  ./dist/glm-4-9b-chat-MLC/ --device "cuda" --model-lib dist/libs/glm-4-9b-chat/glm-4-9b-chat-cuda.so
[2024-06-08 07:04:34] INFO auto_device.py:79: Found device: cuda:0
[2024-06-08 07:04:34] INFO engine_base.py:143: Using library model: dist/libs/glm-4-9b-chat/glm-4-9b-chat-cuda.so
[07:04:34] /workspace/mlc-llm/cpp/tokenizers/tokenizers.cc:154: Warning: Tokenizer info is not detected as tokenizer.json is not found. The default tokenizer info will be used.
[07:04:34] /workspace/mlc-llm/cpp/tokenizers/tokenizers.cc:130: Warning: Using tokenizer.model since we cannot locate tokenizer.json.
It is recommended to use tokenizer.json to ensure all token mappings are included, since currently, files like added_tokens.json, tokenizer_config.json are ignored.
Consider converting tokenizer.model to tokenizer.json by compiling the model with MLC again, or see if MLC's huggingface provides this file.
[07:04:34] /workspace/mlc-llm/cpp/serve/config.cc:649: Under mode "local", max batch size will be set to 4, max KV cache token capacity will be set to 8192, prefill chunk size will be set to 2048.
[07:04:34] /workspace/mlc-llm/cpp/serve/config.cc:649: Under mode "interactive", max batch size will be set to 1, max KV cache token capacity will be set to 131072, prefill chunk size will be set to 2048.
[07:04:34] /workspace/mlc-llm/cpp/serve/config.cc:649: Under mode "server", max batch size will be set to 80, max KV cache token capacity will be set to 314995, prefill chunk size will be set to 2048.
[07:04:34] /workspace/mlc-llm/cpp/serve/config.cc:729: The actual engine mode is "interactive". So max batch size is 1, max KV cache token capacity is 131072, prefill chunk size is 2048.
[07:04:34] /workspace/mlc-llm/cpp/serve/config.cc:734: Estimated total single GPU memory usage: 13215.253 MB (Parameters: 5043.234 MB. KVCache: 5202.672 MB. Temporary buffer: 2969.346 MB). The actual usage might be slightly larger than the estimated number.
[07:04:36] /workspace/mlc-llm/cpp/tokenizers/tokenizers.cc:154: Warning: Tokenizer info is not detected as tokenizer.json is not found. The default tokenizer info will be used.
[07:04:36] /workspace/mlc-llm/cpp/tokenizers/tokenizers.cc:130: Warning: Using tokenizer.model since we cannot locate tokenizer.json.
It is recommended to use tokenizer.json to ensure all token mappings are included, since currently, files like added_tokens.json, tokenizer_config.json are ignored.
Consider converting tokenizer.model to tokenizer.json by compiling the model with MLC again, or see if MLC's huggingface provides this file.
sentencepiece_processor.cc(922) LOG(ERROR) 3rdparty/tokenizers-cpp/sentencepiece/src/sentencepiece_processor.cc(289) [model_] Model is not initialized.
Returns default value 0
[07:04:36] /workspace/mlc-llm/cpp/tokenizers/tokenizers.cc:154: Warning: Tokenizer info is not detected as tokenizer.json is not found. The default tokenizer info will be used.
[07:04:36] /workspace/mlc-llm/cpp/tokenizers/tokenizers.cc:130: Warning: Using tokenizer.model since we cannot locate tokenizer.json.
It is recommended to use tokenizer.json to ensure all token mappings are included, since currently, files like added_tokens.json, tokenizer_config.json are ignored.
Consider converting tokenizer.model to tokenizer.json by compiling the model with MLC again, or see if MLC's huggingface provides this file.
You can use the following special commands:
/help               print the special commands
/exit               quit the cli
/stats              print out stats of last request (token/sec)
/metrics            print out full engine metrics
/reset              restart a fresh chat
/set [overrides]    override settings in the generation config. For example,
/set temperature=0.5;top_p=0.8;seed=23;max_tokens=100;stop=str1,str2
Note: Separate stop words in the stop option with commas (,).
Multi-line input: Use escape+enter to start a new line.
Exception in thread Thread-1:
Traceback (most recent call last):
File "/home/haoli/anaconda3.11-GPU_new_mlc2/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
self.run()
File "/home/haoli/anaconda3.11-GPU_new_mlc2/lib/python3.11/threading.py", line 975, in run
self._target(self._args, self._kwargs)
File "tvm/_ffi/_cython/./packed_func.pxi", line 332, in tvm._ffi._cy3.core.PackedFuncBase.call
File "tvm/_ffi/_cython/./packed_func.pxi", line 263, in tvm._ffi._cy3.core.FuncCall
File "tvm/_ffi/_cython/./packed_func.pxi", line 252, in tvm._ffi._cy3.core.FuncCall3
File "tvm/_ffi/_cython/./base.pxi", line 182, in tvm._ffi._cy3.core.CHECK_CALL
File "/home/haoli/anaconda3.11-GPU_new_mlc2/lib/python3.11/site-packages/tvm/_ffi/base.py", line 481, in raise_last_ffi_error
raise py_err
File "/workspace/mlc-llm/cpp/serve/threaded_engine.cc", line 182, in mlc::llm::serve::ThreadedEngineImpl::RunBackgroundLoop()
File "/workspace/mlc-llm/cpp/serve/engine.cc", line 619, in mlc::llm::serve::EngineImpl::Step()
File "/workspace/mlc-llm/cpp/serve/engine_actions/new_request_prefill.cc", line 116, in mlc::llm::serve::NewRequestPrefillActionObj::Step(mlc::llm::serve::EngineState)
File "/workspace/mlc-llm/cpp/serve/model.cc", line 232, in mlc::llm::serve::ModelImpl::BatchPrefill(tvm::runtime::ObjectRef const&, std::vector<long, std::allocator > const&, std::vector<int, std::allocator > const&)
tvm._ffi.base.TVMError: Traceback (most recent call last):
7: mlc::llm::serve::ThreadedEngineImpl::RunBackgroundLoop()
at /workspace/mlc-llm/cpp/serve/threaded_engine.cc:182
6: mlc::llm::serve::EngineImpl::Step()
at /workspace/mlc-llm/cpp/serve/engine.cc:619
5: mlc::llm::serve::NewRequestPrefillActionObj::Step(mlc::llm::serve::EngineState)
at /workspace/mlc-llm/cpp/serve/engine_actions/new_request_prefill.cc:116
4: mlc::llm::serve::ModelImpl::BatchPrefill(tvm::runtime::ObjectRef const&, std::vector<long, std::allocator > const&, std::vector<int, std::allocator > const&)
at /workspace/mlc-llm/cpp/serve/model.cc:232
3: tvm::runtime::PackedFuncObj::Extractor<tvm::runtime::PackedFuncSubObj<tvm::runtime::relax_vm::__mk_TVM8::{lambda(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue)#1}> >::Call(tvm::runtime::PackedFuncObj const, tvm::runtime::relax_vm::__mk_TVM8, tvm::runtime::TVMRetValue)
2: tvm::runtime::relax_vm::PagedAttentionKVCacheObj::BeginForward(tvm::runtime::ShapeTuple const&, tvm::runtime::ShapeTuple const&, tvm::runtime::Optionaltvm::runtime::ShapeTuple const&)
1: tvm::runtime::relax_vm::PagedAttentionKVCacheObj::ReserveAppendLengthInSeq(tvm::runtime::relax_vm::Sequence, long)
0: _ZN3tvm7runtime6deta
File "/workspace/tvm/src/runtime/relax_vm/paged_kv_cache.cc", line 2004
TVMError: Check failed: append_length > 0 (0 vs. 0) : Append with length 0 is not allowed.

@lihaofd thanks for reporting. We'll look into the issue on glm 4. Meanwhile, would you mind confirming if this issue does not happen for other models like llama?

@MasterJH5574
I have tried  chatglm3-6b, it will not show error "TVMError: Check failed: append_length > 0 (0 vs. 0) : Append with length 0 is not allowed.", but the output is abnormal like
mlc_llm chat  ./dist/chatglm3-6b-MLC/ --device "cuda" --model-lib dist/libs/chatglm3-6b/chatglm3-6b-cuda.so

please introduce shanghai

My name is xxx, and I am a  school school

I- q is a 在校学生 .进行的进行的

I am a language model,
And here's largest
No 'big' what
The speech speech
懈口令公 quo
I'm wrong
您傘
您遮 Aut兼任
quality
语言

@MasterJH5574
I also tried https://huggingface.co/shenzhi-wang/Llama3-8B-Chinese-Chat
seems like below CLI work with commone output
mlc_llm chat  ./dist/Llama3-8B-Chinese-Chat-MLC/ --device "cuda" --model-lib ./dist/libs/Llama3-8B-Chinese-Chat/Llama3-8B-Chinese-Chat-q4f16_1-cuda.so

introduce shanghai
Shanghai, the "Pearl of the Orient," is an iconic metropolis that effortlessly blends traditional Chinese culture and history with its modern, cosmopolitan flair. Located on the eastern coast of China, this vibrant city is one of the most populous and culturally significant urban centers in the world.

As part of the Yangtze River Delta, Shanghai has been an essential trading point for centuries, eventually emerging as a major political and economic hub in the People's Republic of China. It's home to a magnitude of historical, cultural, and architectural marvels that span across eras and styles – the modern skyscrapers of the stunning skyline aptly complemented by tranquil Chinese pavilions and gardens.
From world-renowned attractions like the Shanghai Tower and the iconic Bund, to the restored classical charm of the old French Concession and the beckoning call of sweet juicy sheng jian bing (crispy-skinned pancake) and a steaming pot of xiaolongbao (soup-filled dumplings), this city is a must-visit destination for anyone eager to explore the unique dynamism of contemporary China. With its people, food, and riveting history, Shanghai is sure to meet and, likely, exceed the expectations of curious and keen travelers.

@lihaofd Thanks for sharing so much information. We'll look into this.

internlm2  meet this error.
v0.9.dev0/mlc_ai_nightly_cu122-0.15.dev519-cp310-cp310-manylinux_2_28_x86_64.whl

authors: Hzfengsy MasterJH5574 Ubospica felixslu lihaofd

repo: mlc-ai/mlc-llm


text:
使用autocut提取字幕剪辑视频出现TypeError: silero_vad() got an unexpected keyword argument 'trust_repo'不能正常剪辑视频

用的是autocut test目录下的测试视频，发现也无法正常剪辑视频

https://github.com/pytorch/pytorch/blob/master/torch/hub.py#L487
看下是不是版本不对，trust_repo 是在 1.12 版本加入的

repo: mli/autocut


text:
Support batch_size for encoder/decoder in the NMT example

Support batch_size for the encoder/decoder. The train_batch_size is still 1 but the encoder/decoder support arbitrary batch_size now.

@astonzhang

Thank you Xingjian!

authors: astonzhang sxjscience

repo: mli/gluon-tutorials-zh


text:
add 'global_namespace_import' => true to config

https://mlocati.github.io/php-cs-fixer-configurator/#version:2.18|configurator|fixer:global_namespace_import

I would like to see the results of applying this modification in one of our projects first.

authors: SimBig spawnia

repo: mll-lab/php-cs-fixer-config


text:
Remove depeendency on horovod from accuracy-squad.py script

Removed depeendency on horovod from accuracy-squad.py script
v0.7/language/bert/create_squad_data.py is identical to the original one except for commenting-out unnecessary imports so that users don't need to install horovod, which is very heavy-weight.

@christ1ne  @guschmue  FYI, I believe that this change will help MLPerf Inference submitters to use this accuracy script more easily. Thanks!

agree. @guschmue ?

repo: mlperf/inference


text:
Errors while trying to run android application

Hello. I am trying to use this project to bootstrap an android application. It runs perfectly fine on web, but trying to build and run it on android brings the following error:
✔ Copying web assets from out to android/app/src/main/assets/public in 251.37ms
✔ Creating capacitor.config.json in android/app/src/main/assets in 5.13ms
⠧ copy android [info] Inlining sourcemaps
✔ copy android in 413.24ms
✔ Updating Android plugins in 45.72ms
[info] Found 1 Capacitor plugin for android:
       @capacitor/status-bar@4.0.1
✔ update android in 236.16ms
[error] native-run failed with errors

        ERR_UNSUITABLE_API_INSTALLATION: No suitable API installation found. Use --sdk-info to reveal missing packages
        and other issues.
        1: undefined

        More details for this errors may be available online:
        https://github.com/ionic-team/native-run/wiki/Android-Errors

Can anyone help me with this? I really need it. Thank you.
Also, I think this project is really amazing, and I would love to use it more.

Try reinstalling Android Studio and try again. Sounds like your installation is a bit broken there.

Yeah, you'll need to update Android studio, and ensure you've set the ANDROID_HOME env variable in your shell, e.g.
# Android
export ANDROID_HOME="$HOME/Library/Android/sdk"

authors: mlynch nathanchapman ndungu-mbau


text:
No Releases There

Please look into it

Please look into it

Will do.

Thanks Waiting for it

https://github.com/mmatongo/KISSWSL/releases/tag/2021.7-9
There you go.

Thank you so much

authors: janimaharsh mmatongo

repo: mmatongo/KISSWSL


text:
Switch NT specification to capable_of relationship to GO NT secretion term [sf#7]

Reported by djs93 on 2013-05-29 21:18 UTC
It was agreed at a recent CL meeting to switch specification of neurotransmitter to use a capable_of relationship to new subclasses of NT secretion.
Dependency: Needs to wait for GO to finish adding the relavant terms - see this ticket
Note - CL will follow suit (they currently use more general GO terms).

Updated by djs93 on 2013-07-12 11:21 UTC

status: open --> closed

Commented by djs93 on 2013-07-12 11:21 UTC
Fixed.


text:
"COMPONENT_SET" (component variants) unsupported

When attempting to access a project with component variants, application raises the error:
FigmaParser exception: Non supported object type: "COMPONENT_SET"
App data:
Figma to QML generator, version: 1.0.0, Markus Mertama 2020

I have this issue too how can i fix it .
my figma project link: https://www.figma.com/file/Q4Bcducycs4K4DoOI6WJQ8/Untitled?node-id=0%3A1

repo: mmertama/FigmaQML


text:
author_info not working on posts

Hi,
I'm really not an expert with all of this, so excuse me if this is a stupid one.
I managed to move my blog from Wordpress to github pages and I'm using this theme here.
https://davidobrien1985.github.io
https://github.com/davidobrien1985/davidobrien1985.github.io
Now my _config.yml has also this in it:
defaults:
  -
    scope:
      path: ""
      #type: posts
    values:
      layout: single
      read_time: true
      author_profile: true
      share: true
      comments: true
      related: true

However, the author_info only shows up on the index.html and on sites under _pages not on any of my posts.
I checked the source on the resulting websites (example: https://davidobrien1985.github.io/2016/02/conference-session-recordings-from-nicconf/ ) and it tries to display the author_info however all the variables seem to be empty?
What am I missing?
Cheers
David

Are you sure you have completed all the author info that is available in _config.yml (e.g. twitter username etc)?

@hkalant check https://github.com/davidobrien1985/davidobrien1985.github.io/blob/master/_config.yml
This post shows the author_info now:
https://raw.githubusercontent.com/davidobrien1985/davidobrien1985.github.io/master/blog/_posts/2016-02-26-conference-session-recordings-from-nicconf.md or https://davidobrien1985.github.io/2016/02/conference-session-recordings-from-nicconf/
All of the others don't, except for the index.html on https://davidobrien1985.github.io/
It's all the same _config.yml so no idea why it's not working on most files.

@davidobrien1985 You're pretty close to getting it working but I spotted one small thing.
In your posts you have author: "David O'Brien" which is enabling the author override feature. Essentially what's going on is in each post/page you have author: set with a string, the theme is looking for a corresponding key in _data/authors.yml to assign all of the sidebar stuff to. Since that doesn't exist for you, you get all the blank stuff.
To fix it you have a couple of options.
1. Create authors.yml with something like the following and place it in _data
# authors.yml
David O'Brien:
  name             : "David O'Brien"
  avatar           : "me.png"
  bio              : "MVP for Cloud and Datacentre management. Interested in all things automation."
  location         : "Melbourne Australia"
  email            : "me@david-obrien.net"
  github           : "davidobrien1985"
  twitter          : "david_obrien"

2. Remove author: "David O'Brien" from your posts and pages. This will force things to default to the site.author and the strings in _config.yml will be used.
Hope this helps.

ahhh, great. :)
I'll remove author: from my posts and see how I go. I'll update asap.
Cheers!

Fixed, thanks!

authors: davidobrien1985 hkalant mmistakes

repo: mmistakes/minimal-mistakes


text:
Add Homepage link to 404 and improve its apology

This is an enhancement or feature.
Summary
This makes the 404 page more visually appealing as well as more functional, more of a reason to exist.
Context
N/A

Preview:

Any further review? potential merge?

Thanks for the PR but closing this with the wontfix label.
The 404 page here is just an example I'm using for the demo site. It can easily be customized as you have to fit your site's needs.

@mmistakes Honestly I'd say it's better to merge this in. It's certainly an improvement for your docs site, and you know, there could be more 404 links from time to time as the docs are updated and whatever happens...

@iBug Don't think I agree. I'm seeing a slightly reworded error text and a home link that already exists in the masthead.

authors: iBug mmistakes rlaPHOENiX rlaphoenix


text:
Use the default configuration to build library.

I think the conf is missing.
I think this addresses issue https://github.com/mmottl/sqlite3-ocaml/issues/2

Thanks, merged & released.

Per the discussion on here, I'm fine with removing the guards.
My only intent is that this library have some way to nicely warn users (probably by checking during config/compilation) if the sqlite shared library you're linking against has the necessary extensions. And hopefully some guidance on how to easily remedy the situation if it doesn't.

Ok, I've made a quick release (2.0.9) with the guards removed so that others can use the library again.
@rleonid I guess you are still in the process of figuring out a new patch that addresses your issue.  Feel free to submit a pull request whenever you think it's ready.
Thanks to everybody for investigating this!

authors: mmottl rleonid


text:
ENH: better automatic plot titles in plot_compare_evokeds

plot_compare_evokeds(list_of_evokeds, picks='grad')

yields
RuntimeWarning: More than 6 channels, truncating title ...
Two suggested enhancements:

if picks is one of the standard types (eeg, mag, grad...) then the title should reflect the channel type, rather than trying to list all channel names
if specific channels are requested, the truncation should happen with an info message printed to the terminal, not a RuntimeWarning (admittedly subjective, but I think a warning seems too strong for a "we truncated your plot title" message)

I can take this on at the sprint; cc @jona-sassenhagen because of possible interaction with #4746

Agreed!
I think it's fairly independent and I agree with your judgements though. But we will talk about all this at the sprint anyways.

authors: drammock jona-sassenhagen

repo: mne-tools/mne-python


text:
Morph volumetric source estimate--discrepancy in number of vertices

Describe the problem
we (@virvw and @poojaprabhu9) are trying to morph the whole brain volumetric source estimate to fsaverage (as illustrated in example).
After morphing, the number of vertex count vary per subject.
For example:
subject 1,
<VolSourceEstimate  |  21667 vertices, subject : fsaverage1, tmin : -200.0 (ms), tmax : 1200.0000000000002 (ms), tstep : 4.0 (ms), data size : 21667 x 351>
subject 2,
<VolSourceEstimate  |  21197 vertices, subject : fsaverage1, tmin : -200.0 (ms), tmax : 1200.0000000000002 (ms), tstep : 4.0 (ms), data size : 21197 x 351>

what possibly could have gone wrong?
after morphing, why  am i getting different number of vertices for every subject?
Additional context
snippet of the code is as follows,
src = setup_volume_source_space(listsubj[i], mri=aseg_fname, pos=3.,
                                sphere=sphere, mindist=3.,
                                subjects_dir=subjects_dir)
fname_epochs = (wdir + listsubj[i] +
                "/mne_python/EPOCHS/MEEG_epochs_icacorr_signDT8_" +
                condition[c] + '_' + listsubj[i] + "-epo.fif")
epochs = mne.read_epochs(fname_epochs)
epochs.resample(250, npad='auto', window='boxcar', n_jobs=1, pad='edge',
                verbose=None)
epochs.crop(-0.2,1.2)
picks = mne.pick_types(epochs.info, meg=True, eeg=True, stim=False, eog=False)
evoked = epochs.average(picks=picks)

noise_cov= mne.compute_covariance(epochs, tmin=-0.2, tmax=0.,
                                  method=('shrunk','empirical'))
data_cov= mne.compute_covariance(epochs, keep_sample_mean=True, tmin=0.,
                                 tmax=1.2, method=('shrunk','empirical'))

fname_trans= ('/neurospin/meg/meg_tmp/MTT_MEG_Baptiste/mri/%s/mri/T1-neuromag'
              '/sets/COR.fif') % subj
fname_bem = op.join(subjects_dir, '%s' % subj, 'bem',
                    '%s-5120-5120-5120-bem-sol.fif' % subj)
fwd = make_forward_solution(evoked.info, fname_trans, src, fname_bem,
                            mindist=5.0, meg=True, eeg=True, n_jobs=1)

filters = make_lcmv(evoked.info, fwd, data_cov, reg=0.05,
                    noise_cov=noise_cov, pick_ori='max-power',
                    weight_norm='nai')

stcs = apply_lcmv(evoked, filters, max_ori_out='signed')
morph = mne.compute_source_morph(fwd['src'],
                                 subject_from=subj, subject_to='fsaverage',
                                 subjects_dir=subjects_dir)
 stc_fsaverage = morph.apply(stcs)

During forward calculation, vertices can be removed from the source space. If you look at fwd['src'] for each subject, they will probably already have differing numbers of vertices.

Hi there,
I guess what Pooja means is that the end point of morphing to an average
brain volume would be to end up with a common grid for all participants so
we can perform group stats. An unequal number of vertices prevents an easy
work around for this unless we are missing something in our understanding
of volume morphing current implementation.
Let us know, we can help document / explicit things eventually.
Best,
Virginie van Wassenhove
https://brainthemind.com/ https://brainthemind.com/
On Fri, May 3, 2019 at 6:11 PM Eric Larson notifications@github.com wrote:

During forward calculation, vertices can be removed from the source space.
If you look at fwd['src'] for each subject, they will probably already
have differing numbers of vertices.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/6265#issuecomment-489152206,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABYMUIX2UNCFCTS2HWCEMG3PTRP3NANCNFSM4HKTOVGA
.

I think we do have a design issue here. If you morph to fsaverage you should get stcs with the same volume grid to stcs data can be combined for any arithmetic operation.
@larsoner ok for you? I think it's the issue @britta-wstnr reported to me too no?

Thanks Virginie for explanation.
Please check the snippet too, so that it will be sure whether it's a coding issue from my end or it's a design issue.

Seems reasonable to me

it may take some time to be fixed.
@poojaprabhu9 @virvw you can export the stc to a nifti image and then mask it with a volumetric atlas eg using nilearn.
would this help already?
what do you want to do with the stc in fsaverage space?

No, masking won't do .
We conducted some hipp source reconstruction to test a priori working
hypotheses on evoked data, replicated/solved the single-trial regression in
so we're alright with that.
Now, we wish to :

map individual brains on an average vol space (actually we wanted to do
this last year already and were hoping by now we could or could help to
finalize somehow)
conduct hilbert lcmv (for time-frequency analysis) [Pooja has already
nicely played with this for Hipp volume only]
proceed with group stats on "roi" of interest (hipp but also full cortex)

It would be plain simple and elegant to process the full brain in that
manner. I don't see a satisfactory alternative as the questions we are
adressing are sufficiently complex not to add uneeded methodological
complexity. Willing to help for months but somehow need
feedback/synchronize on your side or we can't be useful... :)
@pooja prabhu prabhuppooja@gmail.com: how is it implemented in fieldtrip
(or spm or  brainstorm) ? or @Britta Westner britta.wstnr@gmail.com already
worked on this?
On Sat, May 4, 2019 at 11:35 AM Alexandre Gramfort notifications@github.com
wrote:

it may take some time to be fixed.
@poojaprabhu9 https://github.com/poojaprabhu9 @virvw
https://github.com/virvw you can export the stc to a nifti image and
then mask it with a volumetric atlas eg using nilearn.
would this help already?
what do you want to do with the stc in fsaverage space?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/6265#issuecomment-489311360,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABYMUITAZDMDTRB2LQZWTL3PTVKEZANCNFSM4HKTOVGA
.

Yes, I got the problems with morphed stc  reported by other people, too. I haven't looked into this yet, but it is on my list. However, I am not too familiar with the whole morphing code yet, so I cannot say anything about how easy a fix this would be ... or where things go wrong.
@virvw : FieldTrip is taking a different approach, where the source grid is set up in MNI space to begin with and then warped to the individual anatomy via the normalized individual brain volume.
The goal is clear, I think: Have a common space to morph to, where each voxel location is the same across subjects and the count of voxels does not change.
If I see it right, @agramfort, this would include figuring out how to drop grid points - or add (?) them - with varying grid counts across subjects: or is there a way to specify the number of grid points in the individual source spaces?

Please, keep Pooja and I in the loop (I may be offline for a couple weeks
due to travels) but let us know how we can help advance!
On Sat, May 4, 2019 at 12:15 PM Britta Westner notifications@github.com
wrote:

Yes, I got the problems with morphed stc reported by other people, too. I
haven't looked into this yet, but it is on my list. However, I am not too
familiar with the whole morphing code yet, so I cannot say anything about
how easy a fix this would be ... or where things go wrong.
@virvw https://github.com/virvw : FieldTrip is taking a different
approach, where the source grid is set up in MNI space to begin with and
then warped to the individual anatomy via the normalized individual brain
volume.
The goal is clear, I think: Have a common space to morph to, where each
voxel location is the same across subjects and the count of voxels does not
change.
If I see it right, @agramfort https://github.com/agramfort, this would
include figuring out how to drop grid points - or add (?) them - with
varying grid counts across subjects: or is there a way to specify the
number of grid points in the individual source spaces?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/6265#issuecomment-489314118,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABYMUIVQY3MPANNNDAZIKRDPTVO3VANCNFSM4HKTOVGA
.

@poojaprabhu9 can you try to output the morphed volumes as nifti files with:
morph.apply(stc, output='nifti1')
You should get nifti1 images in fsaverage space. I hope this helps as otherwise
I'll been someone to look into this. Not sure if @TommyClausner has still time?
Otherwise maybe @larsoner can do black magic?

No @agramfort. This did not help.
On Sun, May 5, 2019 at 8:09 PM Alexandre Gramfort notifications@github.com
wrote:

@poojaprabhu9 https://github.com/poojaprabhu9 can you try to output the
morphed volumes as nifti files with:
morph.apply(stc, output='nifti1')
You should get nifti1 images in fsaverage space. I hope this helps as
otherwise
I'll been someone to look into this. Not sure if @TommyClausner
https://github.com/TommyClausner has still time?
Otherwise maybe @larsoner https://github.com/larsoner can do black
magic?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/6265#issuecomment-489432521,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI6HEXZ4HXGRTT6Y46PUNJLPT3WTHANCNFSM4HKTOVGA
.

--
Thank You
Pooja Prabhu

Any luck?
Actually, the morphing of volume source (whole brain) is a vital part of our work. I would request expertise to suggest some solution.
Thanks

Hey @poojaprabhu9
Sorry must have somehow missed this :/
I'll have a look later today ...

Hey @poojaprabhu9
do both subjects have the same number of vertices when you do ?
stcs = apply_lcmv(evoked, filters, max_ori_out='signed')
Because as far as I can see, I assume the difference of vertices comes from the fact that there are different input vertices. Meaning that compute_source_morph re-scales the existing vertices into target space ('fsaverage'). That is it uses both MRIs (subject + fsaverage) to compute a volumetric morph, that will be applied to the result of apply_lcmv. Hence the difference in number of vertices doesn't seem to be surprising.
To be a bit more clear: the (number of) vertices are actually voxel indices where "activity" can be found. Thus len(stc_fsaverage.vertices) should be equal to len(stc_fsaverage.data). Corresponding regions would thus be those, where the vertex values are the same. So you could define a vector of vertices that has all indices corresponding to the actual grid space and assign the data using stc_fsaverage.vertices from both subjects. Another way is to convert the data into an actual volume
Could you try
morph.apply(stc, mri_resolution=True, output='nifti1')
and see whether the resulting nifti has the shape (256, 256, 256, 1) ?
(Kinda what @agramfort suggested)
Cheers,
Tommy

Hence the difference in number of vertices doesn't seem to be surprising.

This was my reaction at first, too, then I realized that while this is true, it doesn't solve a critical use case.
Let's quickly say what's done in an example surface case:

Every subject starts with an oct6 source space, 8196 vertices total (4098 per hemi)
Some get excluded during forward computation, say for one subject you have 8195 verts, and another one 8194
When you morph to fsaverage you not only morph but also resample/upsample to a standard space (typically a "grade 5", i.e. ico-5 with 10242 per hemi / 20484 total)

Thus at the end of both stc_fs_1 = morph.apply(stc_subj_1) or stc_fs_2 = morph.apply(stc_subj_2) you have a SourceEstimate with 20484 verts, which is very helpful.
For the volume case, this morph is done but the resample/upsample step is a bit hidden in the zooms/spacing selected during morphing, and doesn't show up explicitly in the VolSourceEstimate. You can do the NIfTI output of shape (256, 256, 256, n_times), but this is challenging for many reasons (poor memory usage, no built-in support for MNE functions like there is for (Vol)SourceEstimate, etc.).

Maybe is an additional note:
the default grid space that is used for morphing (as you used in the code you provided is iso 5 mm). Thus the resulting volume is of shape (51, 51, 51, 1), which means that the values found in stc_fsaverage.vertices refer to a location in range(0, 51**3)

Option 1
Assuming you are saying that the data for, say, vertex 12345 for stc_fs_1_vol and stc_fs_2_vol refer to the same voxel in the 5mm-zoom space (right?), if we provided a way to zero pad (or probably better, spatially interpolate) the data for stc_fs_1_vol and stc_fs_2_vol so that they both contained the same set of vertices, then everything would be aligned properly. Meaning stc_fs_1_vol.data rows are the same spatial locations as those in stc_fs_2_vol.data. @TommyClausner do you know the best way to do this interpolation (maybe just nearest neighbor is good enough, or trilinear if some neighbors are almost the same distance away)?
One additional consideration here is that it's sometimes very nice to have the source space corresponding to the source estimate you are using. So if we go this route, we should probably add a morph.get_output_source_space() or similar method that gave the effective MNE volume source space corresponding to the output volume source estimate object.
Option 2
Another option would be to say what source space to resample/upsample to when specifying the volume morph. I spoke briefly to @agramfort and, following the surface path, this could be done by:

Adding a standard fsaverage 5mm spacing volumetric source space (easy with src_fs_vol_5mm = setup_volume_source_space('fsaverage', pos=5.) and some dataset updates)
Making compute_source_morph in the volume case allow specifying a destination source space to morph to, such that once you did stc_fs_1_vol = morph.apply(stc_subj_1_vol) you had a VolSourceEstimate whose stc_fs_1_vol.vertices corresponded to vertices in the complete src_fs_vol_5mm space, i.e., it was always the case that np.array_equal(stc_fs_1_vol.vertices, src_fs_vol_5mm[0]['vertno']).

@TommyClausner how hard would it be to get this to use the src_fs_vol_5mm source space locations, rather than generic ones we currently use? (Or at least some full/complete source space, rather than one with holes?) Maybe we could just take the existing transformation (computed at whatever accuracy/spacing/zooms are being used), then add one more linear / sparse matmul that goes from this space to the src_fs_vol_5mm space / vertices.
I'm not sure which option is better/easier, but I think either one would probably allow @poojaprabhu9 and @virvw to proceed.

hm... Thanks @larsoner  ! Sounds reasonable to make the output such that it is standardized :)

Assuming you are saying that the data for, say, vertex 12345 for stc_fs_1_vol and stc_fs_2_vol refer to the same voxel in the 5mm-zoom space (right?)

Yes, which means a grid like np.zeros((51, 51, 51)) and then plugging in the values should already do the trick, no ?
What I just wonder is the following:
when I call mne.setup_volume_source_space('fsaverage', pos=5.)[0]['shape'] I get (39, 39, 39) and for the standard 5 mm morph (51, 51, 51). So actually the standard fs source space using pos=5. should be a sub-volume of the morph output, right ? (probably excluding outside brain regions, that are in the MRI-like volumetric morph space) - So wouldn't it just be possible to find the corresponding subset, create a new source space (mne.setup_volume_source_space('fsaverage', pos=5.)) and insert the values there / sub-sample morph.apply(stc).vertices to fit the src ?
I guess I miss something, but where is the need for an interpolation ?
btw: guess a trilinear interpolation, should do the trick in case we indeed need it, no ?

So actually the standard fs source space using pos=5. should be a sub-volume of the morph output, right ...  So wouldn't it just be possible to find the corresponding subset,

It depends on whether or not it's actually a subset of identical X/Y/Z coordinate (centers). It's entirely possible that the pos=5. creates the 5x5x5 voxel centers in different places from the "standard 5mm morph". We'd have to dig into the affine to check. But yes hopefully we don't need any interpolation and can just effectively subselect. @TommyClausner can you look into a bit? I always get a bit lost looking at the transformations and code in setup_volume_source_space.

I will just chime in here to re-iterate the two important points:

same number of vertices across subjects
the vertices should fall onto the same brain tissue across subjects

The latter point might only be possible with non-linearly warped grid positions in individual space, i.e., the FieldTrip way of doing things. Alternatively, the fsaverage brain will be non-equidistant, right?
As discussed with @agramfort, this will introduce problems for plotting on MRIs (i.e., never converging interpolations). I would strongly vote for the fsaverage to be the equidistant brain (then individual subjects can still be plotted on this).
I am not sure about interpolating, as this might pose problems for statistics: especially with 5 mm spacing (or coarser) this can place grid points in the average at different places than in the individual subjects.

same number of vertices across subjects
the vertices should fall onto the same brain tissue across subjects

It depends on when in the pipeline you want this to occur. The way we have been talking about things (and how it works in master), each subject source space would have a variable number of verts (mostly head-size/shape dependent), and then after morphing they all have the same source space vertices in fsaverage's space (except for holes, which is what we so far are trying to figure out how to fix).

Alternatively, the fsaverage brain will be non-equidistant, right?

In either of the two options I mention above, after morphing the VolSourceEstimate vertices will all be in the same space (fsaverage with some even grid spacing).

The latter point might only be possible with non-linearly warped grid positions in individual space, i.e., the FieldTrip way of doing things.

It sounds like you want to take a fsaverage source space, warp it to subjects, then do source localization. I don't think this solves the problem of "holes" due to forward exclusion -- the fs->subj morphed source space can have vertices culled from it just like the individual subject source spaces can. Either way you need to either interpolate the missing vertices for subjects missing them, or ignore vertices for all subjects that were missing from any individual subject.
FYI what I think you're suggesting we have the equivalent for in surface source space as morph_source_spaces, but it doesn't get used often and poses its own challenges. So for consistency it would be nice not to go that route unless we have to.
Also thinking about topology and localization -- it might not be as good for source estimation to use less-evenly-spaced sources during the localization step (which would happen taking an evenly spaced fsaverage source space and warping it). It might be preferable to deal with the spacing differences after localization via proper interpolation in the destination space. But maybe this is backward thinking...?

I am not sure about interpolating, as this might pose problems for statistics: especially with 5 mm spacing (or coarser) this can place grid points in the average at different places than in the individual subjects.

You can always interpolate onto a finer grid if you want (down to even 1mm). On surfaces we already do this typically by upsampling 8196 source points onto 20484 on fsaverage. I'd guess that 5 mm is probably "good enough" given that smoothing is often used/useful for group studies anyway, used in fMRI which has better resolution than us (with Gaussian FWHMs around this size or bigger), and given the assumed M/EEG source resolution. But either way, the option for finer resolution should be there in either option I outlined above.

we support both options with surfaces: 1/ morphing by reinterpolation 2/
using a morphed source space (similar to fieldtrip approach)
I think we should not treat the volumes differently.

using a morphed source space (similar to fieldtrip approach)
I think we should not treat the volumes differently.

Okay for now / in this issue let's focus on fixing the current way of doing things 1/ using one or both of the options I've talked about above, and @britta-wstnr feel free to open an issue for 2/.

I could check out whether it's possible to just "sub-sample" and in case not dive a bit into the ifs and hows on options 1 and 2 that @larsoner suggested

Hi @TommyClausner,
after trying this: stcs = apply_lcmv(evoked, filters, max_ori_out='signed')
Each subject have a different number of vertices (len(stcs.vertices)).
I tried---morph.apply(stc, mri_resolution=True, output='nifti1')
--the resulting nifti has a shape(256,256,256,(number of time points)). i have considered entire time so the number of time points is 281.

Hey @poojaprabhu9
ok cool, but then it should (at least for now) fix the issue of having the stuff in the same space, because now your data for both subjects should be non-linearly morphed towards the "fsaverage"-space and thus be comparable, no ?

@TommyClausner Non-linearly morphing might help. But i am not sure how can i do it?

I mean after calling morph.apply(stc, mri_resolution=True, output='nifti1') the output nifti is a non-linear morph of the source data you were inputing when using compute_source_morph - so the result is a nifti in 'fsaverage' space containing the individual, non-linear morphed subject data

Yes that's right.
On Tue, May 14, 2019, 18:39 Tommy Clausner notifications@github.com wrote:

I mean after calling morph.apply(stc, mri_resolution=True,
output='nifti1') the output nifti is a non-linear morph of the source
data you were inputing when using compute_source_morph - so the result is
a nifti in 'fsaverage' space containing the individual, non-linear morphed
subject data
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mne-tools/mne-python/issues/6265?email_source=notifications&email_token=AI6HEX6I7XAHVGWOZRD4SW3PVK2ZJA5CNFSM4HKTOVGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVLNOJA#issuecomment-492230436,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AI6HEXZGFIJWG52DDRHEVRLPVK2ZJANCNFSM4HKTOVGA
.

Hey @larsoner
I was tinkering around a bit, but I guess I don't fully understand how the source-space spacing is thought...
I tried to create a source space that has the size of the morphed output (similar to when I output it as a nifti), but there are issues:
somehow some axis are swapped (0 and 2) and there seems to be an offset between the source space and the nifti, even though it seems at the first glance pretty straight forward, since creating a volume source space and converting to nifti use the same kind of interpolator. So in theory there should be some kind of straight forward way to create a source space after morphing that just suits the morphed source estimate...
Here is what I did
import os
import mne
from mne.datasets import sample
from mne.minimum_norm import apply_inverse, read_inverse_operator
import numpy as np

sample_dir_raw = sample.data_path()
sample_dir = os.path.join(sample_dir_raw, 'MEG', 'sample')
subjects_dir = os.path.join(sample_dir_raw, 'subjects')

fname_evoked = os.path.join(sample_dir, 'sample_audvis-ave.fif')
fname_inv = os.path.join(sample_dir, 'sample_audvis-meg-vol-7-meg-inv.fif')

fname_t1_fsaverage = os.path.join(subjects_dir, 'fsaverage', 'mri',
                                  'brain.mgz')

evoked = mne.read_evokeds(fname_evoked, condition=0, baseline=(None, 0))
inverse_operator = read_inverse_operator(fname_inv)

# Apply inverse operator
stc = apply_inverse(evoked, inverse_operator, 1.0 / 3.0 ** 2, "dSPM")

# To save time
stc.crop(0.09, 0.09)

morph = mne.compute_source_morph(inverse_operator['src'],
                                 subject_from='sample', subject_to='fsaverage',
                                 subjects_dir=subjects_dir)

stc_fsaverage = morph.apply(stc)

full_mri_size_fs = 256.
size_pos = 5.
sphere_radius = (full_mri_size_fs - 2 * size_pos) / 2
src_fs_vol_5mm = mne.setup_volume_source_space('fsaverage', sphere=(0., 0., 0., sphere_radius), subjects_dir=subjects_dir,  mri='brain.mgz', pos=size_pos)

src_fs_vol_5mm[0]['inuse'] = np.zeros(src_fs_vol_5mm[0]['inuse'].shape)
src_fs_vol_5mm[0]['inuse'][stc_fsaverage.vertices] = 1

tmp_array = np.zeros((np.prod(morph.shape), 1))
tmp_array[stc_fsaverage.vertices] = stc_fsaverage._data
tmp_array = tmp_array.reshape(morph.shape)
tmp_array = tmp_array[::-1,::,::]
tmp_array = tmp_array[::,::,::-1]
tmp_array = np.swapaxes(tmp_array, 0, 2)

stc_fsaverage._data = tmp_array.reshape(np.prod(morph.shape), 1)[stc_fsaverage.vertices]
stc_fsaverage.data = stc_fsaverage._data

stc_fsaverage.plot(src_fs_vol_5mm, subject='fsaverage',subjects_dir=subjects_dir)

stc.plot(inverse_operator['src'], subject='sample', subjects_dir=subjects_dir)

So as you can see, to get it kinda right, I had to swap some axis and reverse the order of some values, but this of course doesn't seem to be the "correct" way... So maybe you or @agramfort or @britta-wstnr  or someone else might have an idea about that... Do you ?
I think the solution to the problem in general is:

morphing as it is now
creating the corresponding fsaverage source space according to the morph settings
arranging the morphed data such that vertices (and data) fit with the new "pseudo morphed" space

Think this would be viable ?
Unfortunately I have not too much time to dig much deeper (unless you might be able to give me a hint where to maybe look at :) )

So as you can see, to get it kinda right, I had to swap some axis and reverse the order of some values, but this of course doesn't seem to be the "correct" way... So maybe you or @agramfort or @britta-wstnr or someone else might have an idea about that... Do you ?

In the MNE volume code there are these transforms like mri_voxel->mri etc. One of them probably has such a transformation/axis swap. Swapping 0 and 2 sounds suspiciously like what you'd do to take a SAR (superior, anterior, right) representation of the data to a RAS. The former is maybe how the data are stored on disk, thinking in C-major order (S/z-slices, then A/y-slice within that z, then R/x within that y and z) and the latter is what MNE/Freesurfer prefers (RAS orientation). But this is just speculation on my part.
What might help is if we start annotating the MNE volume source space setup code with what we understand it's doing. If you already have some of this in your head, feel free to open a WIP PR adding comments about it. If not, I can look at the code and try to annotate it a bit to see if it helps you understand what needs to be done. I'm optimistic that doing so will make it clear when axes need to be swapped and why, so that there will be no more mysterious changes in your code.

I think the solution to the problem in general is:

morphing as it is now
creating the corresponding fsaverage source space according to the morph settings
arranging the morphed data such that vertices (and data) fit with the new "pseudo morphed" space

Think this would be viable ?

As long as by "arranging the morphed data" you mean dealing with the problem of subject-by-subject variable "holes" by some suitable interpolation scheme such that in the end you always get a VolSourceEstimate defined in the "complete" fsaverage source space, yes this sounds good to me.

Swapping 0 and 2 sounds suspiciously like what you'd do to take a SAR (superior, anterior, right) representation of the data to a RAS

hm... Sounds plausible :)

What might help is if we start annotating the MNE volume source space setup code with what we understand it's doing

d'accord !

As long as by "arranging the morphed data" you mean dealing with the problem of subject-by-subject variable "holes" by some suitable interpolation scheme such that in the end you always get a VolSourceEstimate defined in the "complete" fsaverage source space

Since the interpolators in morph and src work after the same principle I hope that there is a way of setting up a source space such, that the source space interpolator just mapps it "right" (not 100% sure that this is the case / will work, but it seems so)
I'll open a WIP PR asap :)

FYI @TommyClausner in #6046 I fixed some stuff that was wrong with the stc.vertices that came from SourceMorph, see this comment. Can you see if your script can be simplified now?

I tried it. it seems that the problem is till intact.
I used the mne 0.19.0.

Yes we have not fixed this yet

Okay @TommyClausner I might have a bit more insight into this now, I think I've come to see some of the things you pointed out above. Sorry in advance for the wall of text. I'll try to make a PR sometime next week to add code comments to the volume source space code to make sure all this is accurate.
If we look at where the sample source space in the inverse comes from:
>>> inverse_operator['src']
<SourceSpaces: [<volume, shape=(21, 26, 24), n_used=3757, coordinate_frame=head>]>

We can recreate this with:
>>> bem = mne.read_bem_solution('/Users/larsoner/mne_data/MNE-sample-data/subjects/sample/bem/sample-5120-bem-sol.fif')
>>> mne.setup_volume_source_space('sample', pos=7., bem=bem, subjects_dir=subjects_dir, verbose=True)
...
Surface extent:
    x =  -66.7 ...   68.8 mm
    y =  -88.0 ...   79.0 mm
    z =  -44.5 ...  105.8 mm
Grid extent:
    x =  -70.0 ...   70.0 mm
    y =  -91.0 ...   84.0 mm
    z =  -49.0 ...  112.0 mm
13104 sources before omitting any.
3757 sources remaining after excluding the sources outside the surface and less than    5.0 mm inside.
>>> src
<SourceSpaces: [<volume, shape=[21 26 24], n_used=3757, coordinate_frame=MRI (surface RAS)>]>

Note that np.prod([21 26 24]) == 13104 -- in other words, stc.vertices, which come from src[0]['vertno'], are indices into the subset of grid vertices contained within the sub-cube (of what would be the original grid of size (51, 51, 51)) defined the "grid extent"s given above.
Then looking at what we want the canonical 5mm-spacing fsaverage destination volumetric source to be we have:
>>> bem_fs = mne.make_bem_model('fsaverage', 5, (0.3,), subjects_dir=subjects_dir)
>>> bem_fs = mne.make_bem_solution(bem_fs)
>>> src_fs =  mne.setup_volume_source_space('fsaverage', pos=5., bem=bem_fs, subjects_dir=subjects_dir, verbose=True)
...
Grid extent:
    x =  -80.0 ...   80.0 mm
    y = -115.0 ...   75.0 mm
    z =  -75.0 ...   90.0 mm
...
43758 sources before omitting any.
...
14629 sources remaining after excluding the sources outside the surface and less than    5.0 mm inside.

At the end of the day, we want our stc_morphed_properly.vertices to correspond to src_fs[0]['vertno'], i.e. indices into the subset of the volume within the given grid extent used during creation of src_fs.
In principle IIUC this should "just" require properly reindexing the stc_morphed_currently.vertices that morph.apply produces -- which has stc_morphed_currently.vertices corresponding to the (51, 51, 51) space -- to vertices corresponding to those given by the "grid extent" of src_fs. This assumes that the voxel centers are the same for both spaces (which I'm not sure about) -- if they aren't, we're talking about trilinear interpolation, basically.
If this sounds reasonable, one option would be to:

Do morphing as we do now
Write a reindexing/resampling function -- keeping in mind that vertno is really reshaped in Fortran order, which is a bug I found and fixed in #6046 -- and see if it gives the expected result when plotting. If the voxel centers are the same in src_fs and in the morph/zooms-based code, I think this is just a reindexing problem. If they aren't, then I think it's a trilinear interpolation problem that we can hopefully solve using nibabel or dipy trilinear interpolation code as a first pass, and the built-in MNE trilinear interpolation -> sparse matrix code for speed eventually. Probably call this resample_to_vol_src: take a stc defined on an entire "MRI grid" and turn it into a stc defined on a "source space grid" if you give it the source space to which it should be restricted.
Either allow passing src_to to SourceMorph directly, to have it do this restriction/conversion for you when you call apply, or add a src_to argument to apply, or just make the resample_to_vol_src public.

Alternatively, we could:

Adjust SourceMorph so that the volume it produces is defined in the same space as a stc_fs.as_volume(mri_resolution=False) based on src_fs would be. I think this would require adding src_to as an arg somewhere in the SourceMorph constructor, as we could then inspect it to get 1) the zooms, 2) the domain/shape, and (maybe?) 3) the voxel centers (/affine?) configured correctly such that the image produced by applying the morph is already has the correct grid extents/voxel centers. We currently don't set (2) based on any src but rather on mri. And I'm still not sure about the voxel centers...

My vote is for this second one. In this scheme, if users do src_to=None in SourceMorph, then it's the behavior we have on master. If they pass src_to=src_fs, then people can omit the subject and zooms arg, and everything should "just work" (hopefully) when they do stc_fs = morph.apply(stc) and stc_fs.plot(src_fs)!
If this works, then we can move on to what I think is mostly a separable issue, namely how to deal with any possible "holes" that come up due to vertices near the edges existing for some subjects and not others, i.e. if none of the vertices in the morphed STC actually map into some of the members of src_fs[0]['vertno']. I think this will end up being a pretty easy problem to solve using some form of iterative trilinear extrapolation, but for now we can just live with some values being undefined (maybe set them to nan, and we can change the behavior via kwarg later?).
And from there, hopefully the volumetric morphing will work like the surface-based morphing for people!

@TommyClausner I adapted your script above into gist showing that if we:

Set the sphere size so it makes a shape of (51, 51, 51) (what the SourceMorph does), and
Replace the affine

You can get the stc.plot(src), stc.plot(morph), and stc_fs.plot(src_fs) to all match. This is all consistent with the stuff above. So I'm optimistic that the plans above could work.

authors: TommyClausner agramfort britta-wstnr larsoner poojaprabhu9 virvw


text:
RFC: migrate coreg

I wonder how difficult it would be to migrate to Qt + traitlets instead of using traitsui for mne Corey. Then we can easily use other 3D backends. The "disadvantage" is that we'd lose the GUI backend abstraction, but I don't think wx ever worked that well anyway.
It might also be a lot of work.

mne Corey

mne coreg most likely :)

This is in the roadmap so I'll remove this

authors: larsoner mmagnuski


text:
[BUG] mne coreg GUI saves fiducials in meg space but labels as mri space

Try:
SUBJECT=sample
SUBJECTS_DIR=/path/to/mne_data/MNE-sample-data/subjects/
mne coreg

Save fiducials as test file
import mne
fids, coord_frame = mne.io.read_fiducials('/path/to/saved/mne_sample-fiducials.fif')
print(fids)

The fiducials are in meg space but have coord_frame 5 corresponding to mri space.
[{'kind': 1, 'ident': 1, 'r': array([-0.07629625, -0.00062556, -0.00776012], dtype=float32),
 'coord_frame': 5}, {'kind': 1, 'ident': 2, 'r': array([0.00267222, 0.09362256, 0.03224791], dtype=float32),
'coord_frame': 5}, {'kind': 1, 'ident': 3, 'r': array([ 0.07635873, -0.00258065, -0.01212903],
dtype=float32), 'coord_frame': 5}]

Voxel space is something like:
array([[67.7471001 , 52.83904668, 48.1775196 ],
       [40.75665484, 24.14586344, 64.9023987 ],
       [16.50280451, 56.2472034 , 48.80732148]])

Either the fids should be changed to voxel space or the coordinate system should be changed to head.

Either the fids should be changed to voxel space or the coordinate system should be changed to head.

What we call MRI is the "Surface RAS" used by FreeSurfer documented here:
https://mne.tools/dev/overview/implementation.html?highlight=coordinate frames voxel#meg-eeg-and-mri-coordinate-systems
It's not the same as voxel coordinates. If you want to go from "MRI" to "MRI voxels", you'd want (the inverse of) the img.header.get_vox2ras_tkr() transform from nibabel, e.g.:
https://gist.github.com/larsoner/0ac6fad57e31cb2d9caa77350a9ff366#file-sample_mri_pos-py-L28
FYI that gist was used to get the electrodes from head coords onto the MRI for use in this example:
https://mne.tools/dev/auto_tutorials/source-modeling/plot_eeg_mri_coords.html

Very cool example, my mistake on the coordinate system. Looks good to me!

Ok sorry still a bit confused.
I was using
mri_landmarks = apply_trans(trans, meg_landmarks, move=True) * 1e3

To get to what I thought was the mri space you are referencing.
Then
vox2ras_tkr = t1_mgh.header.get_vox2ras_tkr()
ras2vox_tkr = np.linalg.inv(vox2ras_tkr)
mri_landmarks = apply_trans(ras2vox_tkr, mri_landmarks)  # in vox

to get to voxels.
That worked for the landmarks saved out of the GUI, but skipping the first step did not.
i.e. voxel space
mri_voxel_landmarks = mne.channels.make_dig_montage(
    lpa=[66.08580, 51.33362, 46.52982],
    nasion=[41.87363, 32.24694, 74.55314],
    rpa=[17.23812, 53.08294, 47.01789],
    coord_frame='mri_voxel')

mri space
mri_landmarks = mne.channels.make_dig_montage(
        lpa=[-69.25741411, 10.58946009, -25.00085936],
        nasion=[3.37909386, 94.65942693, 32.25916435],
        rpa=[77.28562067, 12.05367187, -30.24882153],
        coord_frame='mri')

meg/head space
meg_landmarks = mne.channels.make_dig_montage(
    lpa=[-7.13766068e-02, 0.00000000e+00, 5.12227416e-09],
    nasion=[3.72529030e-09, 1.02605611e-01, 4.19095159e-09],
    rpa=[7.52676800e-02, 0.00000000e+00, 5.58793545e-09],
    coord_frame='head')

What I'm confused about is that the fiducials saved out of the mri gui are in meters (<<1) whereas the mri_space is related to voxels (>1). I'll keep working through it but if there's a simple explanation that would help a lot.

Ahhh I think I was just missing a conversion to mm

@alexrockhill now that you understand it thoroughly, care to write a tutorial on coordinate transformations?

I could do that. Is there a tutorial for something else that it would be helpful if I modeled it off @drammock?

Start here: https://mne.tools/dev/auto_tutorials/source-modeling/plot_source_alignment.html
See if it makes more sense to edit / improve that one, or if a separate stand-alone tutorial would be better.

Ok sorry super basic question: it looks like auto_tutorials are maybe in the .gitignore? How do I make a feature branch with this edited?

the source of the tutorials are in /path/to/mnepython/tutorials/.  The rendered versions get put in .../doc/auto_tutorials by sphinx.

authors: alexrockhill drammock larsoner